PHP Application layers

Question

I'm dealing with design layers of an application. Basically what I've so far are:

• Business
• Architecture: Client-Server
• Techonologies: PHP+MySQL, HTML5, JS, 3rd parties APIs

My app

• Data Sources: MySql databases, 3rd party APIs
• Data Layer: Interface with CRUD operations, PDO for accessing DBs, Classes implementing CRUD interface, Classes for communication with APIs
• Repository: Classes & Interfaces with bussiness methods for accessing datasource.
• Business object: value objects and business rules
• Application layer: controls flow between Data Layer and Presentation.
• Presentation layer: html design with javascript & ajax & php code. Interacts with application layer for data retrieval

Implementation

Data Source

1x MySQL Database

1x 3rd party api

Data Layer

interface DSOps { /* * @return array */ function read($sql); /* * @return array */ function readAll($sql); /* * @return boolean */ function save($sql); /* * @return boolean */ function delete($sql); } use PDO; class DSSql implements DSOps { private $bd; public function __construct( PDO $bd) { $this->bd = $bd; } public function read($sql) { $stmt=$this->bd->prepare($sql); $stmt->execute(); $response = $stmt->fetch(PDO::FETCH_ASSOC); return $response; } public function readAll($sql) { $stmt=$this->bd->prepare($sql); $stmt->execute(); $response = $stmt->fetchAll(PDO::FETCH_ASSOC); return $response; } public function save($sql, $params) { $stmt=$this->bd->prepare($sql); return $stmt->execute($params); } public function borrar($sql, $params) { $stmt=$this->bd->prepare($sql); return $stmt->execute($params); } } 

Repositories:

interface QueryClients { /* * @return Client */ public function searchById($id); /* * @return array() */ public function searchAll(); /* * @return int */ public function count(); } class RepoClients implements QueryClients { private $ds; public function __construct(DSOps $_ds) { $this->ds = $_ds; } /* * @return array */ public function searchById($id) { $sql = " SELECT * FROM clients WHERE id=$id;"; $response=$this->ds->read($sql); return $response; } /* * @return array() */ public function searchAll() { $sql = " SELECT * FROM clients; "; $response=$this->searchAll($sql); return $response; } .... } 

I've multiple modules that implements the same design. When implementing these design I have the following doubts:

1. Is correct to form queries at repository classes?
2. Where should I handle SQL errors? Having an SQLError
3. How should I handle access to APIs?
4. As my app has role based access for users, how does this impacts my design/implementation? Based on logged user role I've create different classes using same interface but implementing different queries to database.

Answer 1

SQL injection

First of all, your PDO code is a straw plane that never flies. Using prepare() like that makes very little sense, leaving all your queries widely exposed to SQL injection attacks.

prepare() doesn't protect your queries by itself, making them immune by means of some magic. It helps only if you take out every single variable from your query and substitute it with a placeholder, then prepare your query and then finally execute, adding variables separately.

Therefore, all methods in DSSql should be changed in order to accept two variables - a query and an array with data:

public function read($sql, $params = []) { $stmt=$this->bd->prepare($sql); $stmt->execute($params); $response = $stmt->fetch(PDO::FETCH_ASSOC); return $response; } 

Code duplication

As you may notice, the code in DSSql class' methods is heavily duplicated. Why not to create a basic method to run an arbitrary query, an then call this method with only different return formats?

public function query($sql, $params = []) { $stmt=$this->bd->prepare($sql); $stmt->execute($params); return $stmt; } public function read($sql, $params = []) { return $this->query($sql, $params)->fetch(PDO::FETCH_ASSOC); } public function readAll($sql) { return $this->query($sql)->fetchAll(PDO::FETCH_ASSOC); } public function save($sql, $params) { return $this->query($sql, $params); } 

Existing PHP classes' code reuse

As you may notice, your methods now bear very little difference. Why not to take them out at all and make use of the powerful PDOSTatement class already exists in PHP?

public function searchById($id) { $sql = "SELECT * FROM clients WHERE id=?"; return $this->ds->query($sql, [$id])->fetch(); } public function searchAll() { $sql = "SELECT * FROM clients"; return $this->query($sql)->fetchAll(); } 

ORM

The code in the save() method is not really does the save but rater runs an arbitrary query, which could be quite confusing. If you want a real save, read and such methods, consider using an ORM, a class that constructs SQL queries for you, and in such a case it will be indeed save() or find() with no other meaning

Questions

Is correct to form queries at repository classes?

yes, it's the place where SQL belongs

Where should I handle SQL errors? Having an SQLError

Wherever you are handling all other errors. There is nothing special in SQL errors, handle them the same way as others.

How should I handle access to APIs?

That is somewhat too wide question for a comment, especially without any context given. May be you should ask on Stack Overflow.

As my app has role based access for users, how does this impacts my design/implementation? Based on logged user role I've create different classes using same interface but implementing different queries to database.

Well, this is somewhat a key question. Consider using an existing framework instead of rolling up a your own. Established frameworks already have all you need out of the box, including ORM, a template engine and of course a user management module with role based authorization. I would suggest Symfony as being the most popular PHP framework that encourages best coding practices.