FuelPHP Forums

nlytnmnt

This weekend I was writing a rest controller in 1.4 and observed some odd behavour. It seemed like the $rest_format is ignored when declared in my controller.

<?php

class Controller_Api extends Controller_Rest
{

    public $rest_format = 'json';

    public function get_status()
    {
        return $this->response(array('available' => 'false'));
    }

}

When I make a default request (one without an extension) to /api/status I expect a json response.

Instead, I get the following in my response:

array(1) { ["available"]=> string(5) "false" }

What also make this weird is that it is not xml. I don't have a rest.php config file specified, but i should default to it when $rest_format isn't specified.

Any ideas?

ssiangge

I think it is a bug.

you can change to use :

public $format = 'json';

and it should work.

nlytnmnt

Setting $format to json will fix the problem but it removes the ability to support multiple formats by forcing all requests to respond with the json format.

Is this indeed a bug?

ssiangge

I mean $rest_format is the problem.

If you just want some specific action return json format, you have to change the format before return.

$this->format = 'json';

return $this->response(array('available' => 'false'));

ssiangge

OK. I just found the answer.

create a file rest.php in app/config:

return array(

"ignore_http_accept" => true

);

then you can use $rest_format.

P.S. I'm not from FuelPHP. I'm just one of the user.

ssiangge

For Harro Verton:

In Rest::_detect_format, if

HTTP_ACCEPT = */* ,

rest.ignore_http_accept = false ,

and the controller didn't set $this->format, it will always return xml. It happen because */* will convert to /^.*\/.*$/ and the first _support_formats always is application/xml.

I found that $this->rest_format appear after the code above which mean it could be never reach.

nlytnmnt

just tested it and i can confirm that ignoring the accept header addresses my issue.

thanks

https://github.com/fuel/core/blob/1.4/master/classes/controller/rest.php#L220

WanWizard

The reason for this sequence is that it's considered good practive to use the HTTP ACCEPT header to signal the type of response the client wants.

Ideally you should not restrict the client as to the desired format, and for your REST API it doesn't matter, a REST controller delivers all formats out of the box without you having to do anything.

@ssiange: Just curious, what creates an HTTP_ACCEPT = */* ? I've never seen that before.

ssiangge

I tried to use jQuery to create a simple AJAX call:

$.ajax(

{

'url' : 'http://localhost/'

}

);

and check from the browser Inspector -> Network. I'm using Google Chrome.

WanWizard

Standard jQuery behaviour. If you don't specify an accept, it will accept everything.

Better use

setRequestHeader('Accept','application/json; charset=utf-8');

or

$.ajax({
    dataType: ($.browser.msie) ? "text" : "json",
    accepts: {
        text: "application/json"
    }
});

tito

I feel like if we explicitly define the rest_format variable in our controller then it should overwrite whatever accept headers we're given.

This can easily be adjusted by moving:

// Well, none of that has worked! Let's see if the controller has a default

if ( ! empty($this->rest_format))

{

return $this->rest_format;

}

above the logic that reads the 'rest.ignore_http_accept' and below the logic that reads the request extension (e.g. file.json).

WanWizard

We are not going to change that, if only because that would most likely break all RESTful controllers out there.

It is clearly defined as a "default value", which means it is only used when all other options to determine the format failed.

We believe it is not best practice for force a particular return format, it should be to the client to indicate what data format it understands, the RESTful controller should return the data in that format. Which is why the class is coded the way it is.

If you don't like that, simply enable "ignore_http_accept" in your config, and your configured rest_format will be used. Entirely up to you.

tito

Right, I understand the default value functionality of the api --- however it is misleading (in my opinion) to seemingly completely ignore the variable if ignore_http_accept is false. The code will never even get past that block if Fuel is set to use the accept header.

This has caused some confusion (based on the necessity of OP making this post in the first place).

I have since modified my client code to include my intended format in the accept header (application/json) and Fuel works as intended here. (Which is great)

I do not want to ignore the accept headers because I'm not sure how I can get a nice Debug::dump() to work if ignore_http_accept is set to true. Because (if that was true) internally, the format will default to json, but if I want to hit my /service/controller with a simple get request in my browser I want to be able to see my debug statements -- and if Fuel is returning JSON, this will not print out in browser very nicely at all, as it converts the debug html to json.

I have created a small workaround in my controller to somewhat pipe the output appropriately so that I can see debug statements and also return json.

static function ResolveResponse($response) {

// return the resposne

if(Common::HeaderHas_JSON())

return $response;

else {

return Format::forge($response)->to_json();

}

Is the rest_format variable already set internally and that is why the change I suggested would break backwards compatibility?

WanWizard

What Fuel version are you using? Debugging RESTful API calls from the browser has been adressed in 1.7.

It breaks all RESTful controllers for anyone that have used it they way it was intended: as a last-resort default value. In all my REST controller's for example it is defined, just in case there is no accept header.

If your suggestion was implemented, suddenly all my controllers would return a fixed format, instead of use the accept header. That is completely unacceptable.

The documentation (http://docs.fuelphp.com/general/controllers/rest.html#/format_determination) tells you exactly how the format is determined. as you can see, setting

$this->format = 'json';

is all you have to do to force your controller to use json...

tito

I'm using Fuel 1.7 .

As far as I'm aware I cannot debug restful calls if ignore_http_accept == true , because it will dump the json only of the result -- dumping html as plain text -- not interpreted by my browser correctly because JSON response header is returned.

The only reason this is even a problem at all is the Debug output seems like it's not working as intended.

I had been Debug::dump()ing in my model and what the model returned in the response included this debug dump data.

WanWizard

1.7 doesn't have the improved dump feature, it was introduced in 1.7.1.

1.7 simply does a var_dump() if the returned value in a Response isn't a string, or can't be cast to string.

Just tested it on 1.8/develop, and if the RESTful action's format is set to 'json', it does return json, sets the response to application/json, and shows it in the browser as json (if not in production!).

tito

I actually believe I have 1.7.1 already (that is the one I downloaded when I first started using Fuel about a week ago. However in the Readme.md it says:

Version: 1.7

This is the same with the 1.7.1 I just downloaded from the main site @: http://fuelphp.com/files/download/26

and that one also has

Version: 1.7

Am I downloading the wrong file?

WanWizard

echo \Fuel::VERSION

should give you the correct version (1.7.0, 1.7.1, or 1.8-dev for example).

I was convinced the fix was part of 1.7.1, but it looks like it isn't. So you'll have to switch to 1.8/develop (by checking out the repository), or wait until 1.7.2 is released, which will be towards the end of the month.

FuelPHP Forums

Ask your question about FuelPHP in the appropriate forum, or help others by answering their questions.

Howdy, Stranger!

Categories

In this Discussion