6
\$\begingroup\$

I am trying to write an REST API using php from scratch which I am gonna use to play with Angular.js. It is working fine now and I need your opinions on the things I have done in this code to improve myself.

NOTE: I am not going to use the below code in production and I am not recommending anyone. I am just trying to learn how to write REST API from scratch and I am an absolute beginner to REST.

index.php

<?php header('Access-Control-Allow-Origin: *'); require_once('main/connect.php'); require_once('main/controller.php'); require_once('main/model.php'); $called = explode('/', trim($_SERVER['REQUEST_URI'],'/')); if(count($called) <= 3) { header($_SERVER['SERVER_PROTOCOL']." 400 Bad Request"); die(); } $resource = $called['3']; if(!file_exists('controller/'.$resource.'.php')) { header($_SERVER['SERVER_PROTOCOL']." 404 Not found."); die(); } require('controller/'.$resource.'.php'); $controller = new $resource(); $parameters = array_slice($called, 4); $req_body = file_get_contents('php://input'); $req_body = json_decode($req_body,true); $switch = $_SERVER['REQUEST_METHOD']; switch ($switch) { case 'GET': { $controller->view($parameters, $req_body); break; } case 'POST': { $controller->save($parameters, $req_body); break; } case 'DELETE': { $controller->delete($parameters, $req_body); break; } default: { header($_SERVER['SERVER_PROTOCOL'].' 405 Method Not allowed'); break; } } ?> 

Controller

<?php class Products extends Controller{ public function __construct() { parent::__construct(); $this->loadmodel('model_products'); } public function view($id = "",$parameters) { $product_id = 0; if(isset($id[0])) { $product_id = $id[0]; } if(!is_numeric($product_id)) { header($_SERVER['SERVER_PROTOCOL']." 400 Bad Request"); die(); } $data = ""; if($product_id == 0) { $data = $this->model->getProducts(); } else { $data = $this->model->getSingleProduct($product_id); } $this->set_header('json'); echo json_encode($data); } public function save($id = 0,$body) { $product_id = 0; if(isset($id[0])) { $product_id = $id[0]; } if(!is_numeric($product_id)) { header($_SERVER["SERVER_PROTOCOL"]." 400 Bad Request"); die(); } if(!is_array($body)) { header($_SERVER["SERVER_PROTOCOL"]." 400 Bad Request"); die(); } else { $response = ""; if($product_id == 0) { $response = $this->model->saveProduct($body); } else { $body['id'] = $id; $response = $this->model->saveProduct($body); } $this->set_header('json'); echo json_encode($response); } } public function delete($id) { if(!is_numeric($id[0])) { header($_SERVER['SERVER_PROTOCOL']." 400 BAD Request"); die(); } $this->model->delete($id[0]); $data["status"] = "success"; $this->set_header('json'); echo json_encode($data); } } ?> 

The only endpoint as for now is Root/index.php/products. I am not using mod rewrite currently. Is this right or have I misunderstood REST?

\$\endgroup\$
3
  • 1
    \$\begingroup\$If nothing else, add some empty newlines in there to break the code into related chunks. Think of formatting your code into related blocks, like paragraphs in an essay\$\endgroup\$CommentedApr 1, 2015 at 13:21
  • 1
    \$\begingroup\$check out this excellent StackOverflow thread about post and put: stackoverflow.com/a/630475\$\endgroup\$
    – lilobase
    CommentedApr 1, 2015 at 13:35
  • 1
    \$\begingroup\$And also create a function to handle all your error case, there is a lot of redundancy.\$\endgroup\$
    – lilobase
    CommentedApr 1, 2015 at 13:36

2 Answers 2

6
\$\begingroup\$

Instead of:

$_SERVER['REQUEST_URI'] 

REQUEST_URI:index.php/products/xxx/yyy

Use:

$_SERVER['PATH_INFO'] 

PATH_INFO:/products/xxx/yyy

PATH_INFO is relative to the root URL. REQUEST_URI is not.

This leaves flexibility for the ROOT directory using relative URIs.
You can use various URLs and sub-domains.

URL: http://api.yoursite.com/post/index.php/parameters/

REQUEST_URI: /post/index.php/xxx/yyy<br>

PATH_INFO: /products/xxx/zzz


Would not be a bad idea to include a space in trim().

 $called = explode('/', trim($_SERVER['PATH_INFO'],'\x20\x2f')); 

preg_match_all() could provide error detection and correction (user typo) as will as doing the explode().


I would make the code function correctly within the definition of the rules. Although I would expand on it. I would NOT use the Request Method to determine any functionality.

The API should be architected where GET and POST Types can be used interchangeably. I would NEVER use them to select view, save, delete.


While on the subject of parameters I would suggest using integer parameters over string whenever possible.
Integer is compact keeping down the Request Header size, is the easiest to validate, and makes comparison functions faster.

Example: On a string true false parameter $param = intval($param);, no need to do if(isset()) it's not there it's zero. And it resolves typing between numeric 1 and string '1' when it is used as an array dimension.

Instead of this:

if(isset($id[0])) { $product_id = $id[0]; } if(!is_numeric($product_id)) { header($_SERVER['SERVER_PROTOCOL']." 400 Bad Request"); die(); } $data = ""; if($product_id == 0) { $data = $this->model->getProducts(); } 

Create a product with a product id of zero with a description of 'Invalid Product ID" and process it just like a valid product or process the same as an invalid product id number.

 $product_id = $data = $this->model->getProducts(intval($id[0])); 

If you want a product id to return a list of products use a non-zero id.

For delete, return an error status status on zero the same as if it were an invalid product id.


Consider using an extension other than .php. Even html is easier than php for users to remember.

You could use the extension for the user to select the response type.

For Example: In the Apache config or .htaccess

<FilesMatch *.htm? *.xml *.jsn *.txt *.php > SetHandler php5-script </FilesMatch> 

This is easier on the memory allocation, memory bus I/O, and retyping a variable from string to array.

$req_body = json_decode($file_get_contents('php://input'),true); 

Than:

$req_body = json_decode($file_get_contents('php://input'); $req_body = json_decode($req_body,true); 

This is confusing when the first passed argument is $parameters $controller->view($parameters, $req_body);

And the accepted 2nd argument is$parameters<br>

 function view($id = "",$parameters) 

This is poor on more than one level.

if(count($called) <= 3){ header($_SERVER['SERVER_PROTOCOL']." 400 Bad Request"); die(); } 

Check validity of all passed parameters. Then determine the best, most specific and user friendly response.
The best user response is to fix the error when possible.

If a parameter is missing, add a default.

If you had an HTML error on your web page, do you expect it to render? Would you like it if a single HTML error returned a 404 response? If a Browser was that rigid it may not get much use, same goes for your API.

404 is not an appropriate response when the document URL (index.php) is correct. Additionally this will confuse some users as to why. They may just think your server is down and they go somewhere else.

Consider using an alternate response such as 409. You want to make it as easily as possible for the user to understand what their mistake was. 404 should be reserved for an error in the URL

Also for diagnostic and support issues, use various response codes for different issues and document them for the user.

You could use a text header and a message identifying the specifics problem with an error message.

header('Content-Type: text/plain; charset=utf-8'); echo $error; 

Or follow the HTTP 4xx header with an error code in an additional header key value with the error code.

header("x-error: $error"); 

For Example: The W3C HTML Markup Validator puts the number of HTML errors in the header.

There are much better HTTP Response Codes to use over 404.

Better diagnostics reduces support and increases user satisfaction.

400 Bad Request
The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications.

403 Forbidden
The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead.
404 Not Found
The server has not found anything matching the Request-URI. No indication is given of whether the condition is temporary or permanent. The 410 (Gone) status code SHOULD be used if the server knows, through some internally configurable mechanism, that an old resource is permanently unavailable and has no forwarding address. This status code is commonly used when the server does not wish to reveal exactly why the request has been refused, or when no other response is applicable.

405 Method Not Allowed
The method specified in the Request-Line is not allowed for the resource identified by the Request-URI. The response MUST include an Allow header containing a list of valid methods for the requested resource.

406 Not Acceptable
The resource identified by the request is only capable of generating response entities which have content characteristics not acceptable according to the accept headers sent in the request.

409 Conflict
The request could not be completed due to a conflict with the current state of the resource. This code is only allowed in situations where it is expected that the user might be able to resolve the conflict and resubmit the request. The response body SHOULD include enough

information for the user to recognize the source of the conflict. Ideally, the response entity would include enough information for the user or user agent to fix the problem; however, that might not be possible and is not required.

Conflicts are most likely to occur in response to a PUT request. For example, if versioning were being used and the entity being PUT included changes to a resource which conflict with those made by an earlier (third-party) request, the server might use the 409 response to indicate that it can't complete the request. In this case, the response entity would likely contain a list of the differences between the two versions in a format defined by the response Content-Type.

410 Gone
The requested resource is no longer available at the server and no forwarding address is known. This condition is expected to be considered permanent. Clients with link editing capabilities SHOULD delete references to the Request-URI after user approval. If the server does not know, or has no facility to determine, whether or not the condition is permanent, the status code 404 (Not Found) SHOULD be used instead. This response is cacheable unless indicated otherwise.

410 response is primarily intended to assist the task of web maintenance by notifying the recipient that the resource is intentionally unavailable and that the server owners desire that remote links to that resource be removed. Such an event is common for limited-time, promotional services and for resources belonging to individuals no longer working at the server's site. It is not necessary to mark all permanently unavailable resources as "gone" or to keep the mark for any length of time -- that is left to the discretion of the server owner.

\$\endgroup\$
    3
    \$\begingroup\$

    REST

    It seems that you currently allow something like

    POST /Root/index.php/products/someNewID 

    Generally, when creating an entry for a collection, it should be posted to the collection, not to a specific entry (the id is then determined by your application, not by the user). So it should be:

    POST /Root/index.php/products 

    And for updating an existing entry, you would use PUT:

    PUT /Root/index.php/products/someID 

    Here is a nice overview of what HTTP methods should be used for what.

    Reduce Method Complexity

    Currently, your read and save methods are a bit complex, because they handle the single case and the multiple case (read), or the update and create (save). This leads to multiple checks on the id (isset and != 0). Your code would look better if there would be separate methods for each case, eg:

    public function view($id = "",$parameters) { if(isset($id[0])) { $data = viewOne($id); } else { $data = viewAll(); } $this->set_header('json'); echo json_encode($data); } public function viewAll() { return $this->model->getProducts(); } public function viewOne($id) { if(!is_numeric($id)) { header($_SERVER['SERVER_PROTOCOL']." 400 Bad Request"); die(); } return $this->model->getSingleProduct($id); } 

    Of course, you have the !is_numeric($id) block multiple times, so you should extract it to a separate method.

    Misc

    • 3 and 4 are magic numbers (if I move your code inside a sub-directory, I would need to change it), so I would declare them in a variable at the top of the script.
    • it's customary to name the operations you are performing create, read, update and delete (CRUD) (or if you want to insert, select, update, delete as SQL does it), but you use view for read, and save for create.
    \$\endgroup\$
    1
    • \$\begingroup\$thanks tim. angular doesn't have put method by default. So I decided to use the POST for both create and update. I'll try to extend the angular resource\$\endgroup\$
      – Vignesh
      CommentedApr 1, 2015 at 14:07

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.