Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Features #172

Open
beoss opened this issue Sep 18, 2013 · 6 comments
Open

New Features #172

beoss opened this issue Sep 18, 2013 · 6 comments

Comments

@beoss
Copy link

beoss commented Sep 18, 2013

Hello,

I just created a fork of version 3.2 and added some functionality that may be useful in the core framework. The functionality I added enables @uri annotations on class methods and enables support for all HTTP methods (PUT, POST and DELETE).

Check out the changes in this repo: https://github.com/beoss/tonic

Thomas

@beoss
Copy link
Author

beoss commented Sep 18, 2013

I also just made a change so the Autoloader.php doesn't require the folder to be named "Tonic" so it can be renamed if needed.

@drkibitz
Copy link
Contributor

@beoss I like those changes, and I don't mean to speak for @peej, but it would be great if you got some unit tests behind them.

@peej
Copy link
Owner

peej commented Sep 19, 2013

Hi @beoss

I'm confused about your additions to the Application object. It appears to presume that the request body of the request is a querystring style encoded string and parse it into the PHP REQUEST superglobal. I'm not sure this is a sensible thing to do.

  1. Messing with superglobals is a bad idea.
  2. The request body could be something else than a querystring style encoded string.
  3. Request related code should be in the Request object.

You can do the same things via a method condition on a per content type basis, see the json example in the README.

I also don't see the use case for @uri annotations on class methods but I'm happy to be convinced.

@beoss
Copy link
Author

beoss commented Sep 19, 2013

Hi @peej

My changes to the application constructor do the following for PUT/DELETE operations. PHP doesn't put I/O stream data into any scope with those type of requests (or QueryString for that matter). The changes to Application take any data input in the input stream and add tehm to the $_REQUEST scope so that data is accessible anywhere. An example I can provide is a PUT operation where you have a "id" in your URI pattern to update a record but the data for that update is in the I/O stream, this change makes that possible. It also adds any query string data to this scope because PHP doesn't do that for PUT/DELETE requests. These variables should be accessible globally without a method condition the same way $_GET, $_POST, etc...are but since PHP does not do that this is the fix I implemented. This could be done in a method annotation assuming all methods have that annotation.

The other change I did to application is reading annotations and assigning @uri annotations to methods in readMethodAnnotations. You'll see I added an else if to allow @uri and convert the value to a regular expression. I convert it to a regular expression here so I can perform a preg_match in the Resource lookup method to get the appropriate method to execute, my change only runs if the method has a @uri annotation otherwise it falls back to the priority logic.

I agree about use case tests for @uri conditions, I'll work on adding them. These chagnes are based on work I did with version 3 of Tonic last year for a client application that have been working very well.

So that is the justification for my changes I welcome feedback.

Thanks,
Thomas

@peej
Copy link
Owner

peej commented Sep 20, 2013

Any data in the HTTP request will be read into the Request::$data property (as a string), you can then process this data depending on the request content-type header. See https://github.com/peej/tonic/blob/master/src/Tonic/Request.php#L63

The PHP $_GET and $_POST arrays make some assumptions. $_GET is filled with data correctly only if the querystring is of the name=value format. The $_POST array is filled with data correctly only if the request content type is application/x-www-form-urlencoded.

Tonic doesn't assume that you are using these content types and instead makes the raw data available to you in the Request object so you can do your own decoding. For example, many people are using Json for data passing these days, Tonic makes it easy to take the request data and json_decode it before your Resource starts to do it's thing.

Resources are modelled as a single PHP class. This is the central tenor of Tonic and you will have a hard time convincing me otherwise.

However, if you can't model your resources correctly and must model 2 resources within a single class, you can easily add your own @uri method condition to do the same thing. Something like:

/**
 * @uri /thing.*
 * @uri /thing
 */
class MyResource extends Tonic\Resource {

    /**
     * @method get
     * @uri /thingone
     */
    function oneThing() {
        ...
    }

    /**
     * @method get
     * @uri /thingtwo
     */
    function twoThing() {
        ...
    }

    function uri($uri) {
        if ($uri != $this->request->uri) {
            throw new Tonic\ConditionException;
        }
    }

}

I'm still to see a use-case that this sort of thing is required other than simply because people want Tonic Resources to be more like MVC controllers.

@rpoulin
Copy link

rpoulin commented Sep 20, 2013

Paul,

I am a great supporter of Tonic and support your idea of one class one
object for the same reasons you do. RESTFUL services work this way even if
the rest of the world dances to a different beat with MS and Oracle at the
helm.

Cheers!
Ron

On Fri, Sep 20, 2013 at 9:23 AM, Paul James notifications@git.luolix.topwrote:

Any data in the HTTP request will be read into the Request::$data property
(as a string), you can then process this data depending on the request
content-type header. See
https://github.com/peej/tonic/blob/master/src/Tonic/Request.php#L63

The PHP $_GET and $_POST arrays make some assumptions. $_GET is filled
with data correctly only if the querystring is of the name=value format.
The $_POST array is filled with data correctly only if the request content
type is application/x-www-form-urlencoded.

Tonic doesn't assume that you are using these content types and instead
makes the raw data available to you in the Request object so you can do
your own decoding. For example, many people are using Json for data passing
these days, Tonic makes it easy to take the request data and json_decode it
before your Resource starts to do it's thing.

Resources are modelled as a single PHP class. This is the central tenor of
Tonic and you will have a hard time convincing me otherwise.

However, if you can't model your resources correctly and must model 2
resources within a single class, you can easily add your own @urihttps://github.com/urimethod condition to do the same thing. Something like:

/** * @uri /thing.* * @uri /thing */class MyResource extends Tonic\Resource {

/**     * @method get     * @uri /thingone     */
function oneThing() {
    ...
}

/**     * @method get     * @uri /thingtwo     */
function twoThing() {
    ...
}

function uri($uri) {
    if ($uri != $this->request->uri) {
        throw new Tonic\ConditionException;
    }
}

}

I'm still to see a use-case that this sort of thing is required other than
simply because people want Tonic Resources to be more like MVC controllers.


Reply to this email directly or view it on GitHubhttps://github.com//issues/172#issuecomment-24818220
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants