search

joomla-projects / webservices

Webservices working group repository
GNU General Public License v2.0
16 stars 9 forks source link

Refactoring for better structure #30

Closed chrisdavenport closed 8 years ago

chrisdavenport commented 8 years ago

This is a substantial refactoring of the code and I'm sure it breaks a lot of things. It certainly breaks PUT requests (but that should be an easy fix) and SOAP (that's a harder fix because I'm not familiar with SOAP).

Aims

Api now contains classes that deal exclusively with interaction style. Currently this is just REST and SOAP, but other styles could be added if anyone wants to do that.

Renderer now contains classes that deal exclusively with rendering Resource objects. Resource objects no longer have any code which knows about how they will be rendered. Renderers for HAL + JSON, HAL + XML are included. Plain JSON and XML renderers should also work, but are basically the same as the HAL variants except that the plain JSON renderer renders links as HTTP Link headers rather than embedding them in a "_links" object in the payload. The SOAP + XML renderer probably doesn't work at the moment.

Resource is as before, but there are now separate Resource classes for the home (default) page (ResourceHome) and for items (ResourceItem) and collections (ResourceList).

Service is largely unchanged (although I think it should be renamed to "Provider"), but there are now providers for the Api and Renderer objects.

Type contains the transform classes that used to be in the Api/Hal directory, moved because they are not tied to HAL. All Types are now immutable value objects with a pair of named constructors. Some new types have been added; more will be needed.

Webservices used to contain a huge class (over 2000 lines of code!) which is now a little smaller (1500 lines) so there is still plenty of scope for putting it on a diet. There are now separate classes for Create, Read, Update and Delete. There is also a Profile class which is where most of the code relating to the XML configuration files really belongs. Still plenty of scope for refactoring there.

Other changes

A new XML file has been added: site.contents.1.0.0.xml deals with the default page. The name comes from the IANA-registered "contents" link relation. The site.contact.1.0.0.* files have been renamed to site.contacts.1.0.0.* because we should be using plural names for resources going forward. I want to drop the distinction between site and administrator, so I made changes to site.contacts.1.0.0.xml, but haven't touched the administrator.contact.1.0.0.xml file, which is effectively now deprecated. Eventually we should drop the site|administrator prefix from these files. I have temporarily dropped authorisation from the create and delete operations in site.contacts.1.0.0.xml. This is only because I haven't figured out how to write tests which involve authorisation yet. With this refactoring I'm not sure if authorisation still actually works.

A new property, "content_types", has been added to the config.dist.json file, which is an array of support content types that may be served, in order of preference. Content negotiation is handled by willdurand/Negotiation. The chosen content type determines the renderer class that is loaded (eg. application/hal+json maps to the class Renderer\Application\Haljson). The renderer determines the interaction style so, for example, application/hal+json will set the interaction style to "rest". That probably needs to change, but it works for now.

Users and categories probably don't work right now (I haven't tested them), but it should be just a matter of tweaking the XML files to fix that.

The CLI entry point can do everything that the web entry point can. Content negotiation is handled by willdurand/Negotiation in both cases. For CLI just add a --accept argument with the same format as the web Accept header. CLI also supports --method=[HTTP method] so you can do a POST with --method=POST for example. Of course, HTTP headers don't have any meaning for a CLI client, but you can output them to stdout using --sendHeaders if you need to see them. For testing the JSON output from the cli.php I find jq to be very useful (https://stedolan.github.io/jq/). For testing web requests from the command line you can, of course, use curl, but I find httpie easier to use (http://httpie.org/).

To support the refactoring I wrote a bunch of tests which are in a separate repository (https://github.com/chrisdavenport/webservices-tests) because I didn't want to interfere with the work that Javier was doing. The tests use PHP curl to make web requests and cover both HAL + JSON and HAL + XML, but only for the home page and the contacts resources.

To do

Work to be done: I have a long list, which I'll add as issues if this gets merged. There's plenty of room for improvement and all contributions are welcome.

Please test and report your findings. We need to decide if this is a suitable base for moving forwards.

Kixo commented 8 years ago

nice additions @chrisdavenport . I have noticed that you have added classes for Create, Update, Delete and Read operations but not for Tasks, so in this setup they might not be able to run...

HermanPeeren commented 8 years ago

@Kixo the core of REST is a uniform interface, that is a limited set of actions (mostly: CRUD). With that interface you handle resources, not tasks. To use a RESTful interface you'll have to "translate" tasks to resources. For instance: instead of a 'publish' task you would use some published items resource to which you add (POST) an item. Any task can be "translated" into resources. No extra "tasks" are needed. Thinking in tasks = thinking in remote procedure calls, like is done in SOAP.

wilsonge commented 8 years ago

As an on the side as the current branch is largely stable can we merge that into master before this gets merged please :)

wilsonge commented 8 years ago

You need to re-merge the current state of the development branch into this - you have code that this PR removed https://github.com/joomla-projects/webservices/pull/23 relating to sessions (and i guess everything after that)

wilsonge commented 8 years ago

So a lot of comments - but overall this seems like another good round of refactoring. Having said that I think we need the Profile class to be half-usable before we merge this - that class is doing a lot of nothing at the moment it seems :/

chrisdavenport commented 8 years ago

@wilsonge "You need to re-merge the current state of the development branch into this - you have code that this PR removed #23 relating to sessions (and i guess everything after that)".

I have no idea how to do that other than by manually copying the changes. How do you find out if there are any other changes that need merging? I thought everything was up-to-date.

wilsonge commented 8 years ago 
git remote add upstream https://github.com/joomla-projects/webservices.git
git fetch upstream
git merge upstream/develop

Then solve any conflicted files it names

Basically you add this repo as a remote. Then you can access all the branches by "fetching" the updates from that remote. Then merge all the changes into your branch

chrisdavenport commented 8 years ago

@wilsonge "git remote add upstream https://github.com/joomla-projects/webservices.git git fetch upstream git merge upstream/develop"

Just gives me "Already up-to-date."

javigomez commented 8 years ago

Thanks Chris :smile:

Please do not take TravisCI response in consideration since Acceptance tests will fail until the refactoring allows to run the tests via browser. Regarding Chris tests https://github.com/chrisdavenport/webservices-tests/blob/master/tests/categoriesHalJson.php I will study how they can be run with the rest of the suite and see how can they be integrated.

Thanks