ChiliProject is not maintained anymore. Please be advised that there will be no more updates.

We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.

[Proposal] - REST API change

Added by Eric Davis at 2011-02-11 12:38 am

When doing the code review of the latest upstream commits, I found the entire REST API was rewritten to use custom builder templates. This makes me concerned for a few reasons.

Current REST implementation

(using the XML format as my primary examples but is the same for JSON and YAML)

  1. (unverified) I remember seeing performance problems with builder and larger XML documents. I think this is what caused a bunch of Ruby libraries to implement to_xml methods.
  2. to render the templates a lot of custom code is used which includes some method_missing calls. This feels really excessive to me and could be another performance problem
  3. the templates use hardcoded fields so plugins cannot add their own attributes into the existing API methods
  4. the hardcoded fields also mean that if a model wants to change a field then we have to remember to change the API templates also (e.g. rename field, add field, delete field)
  5. the templates also duplicate the fields: for example compare views/issues/index.api.rsb and views/issues/show.api.rsb

In my opinion there are two things the API should be:

  1. Fast (as in performance)
  2. Easy to maintain along with the rest of the views/models

I don't think the current system of using builder templates will be good in the long term.

Proposal

What I propose is to move back to the to_format methods (to_xml, to_yaml, to_json). These are provided by Rails and every single Rails application API I've worked on has used these names, even with custom APIs.

  • for simple cases, the standard Rails method could work fine (and would make it easy to add a simple API on top of any model with just a respond_to block)
  • for more advanced cases, we can overload to_xml on the model itself to provide a custom format
  • for edge cases in a controller, we can send to_xml a block to customize the output
  • Rails automatically adds to_xml to Arrays so we will automatically get a collection API. Better yet, this will use the model's to_xml which means a collection and single record response will have identical fields
  • using model methods, plugins can patch or hook into the existing methods to add their own fields
  • each installation can swap out the serialization in to_xml if performance is a concern (e.g. replace Rails' version with Nokogiri)
  • using the builtin Rails methods means we don't need all of the custom code to support builder templates for the API

The one major thing we would need to do is to provide a single method for each model that can tell all of the formats how to render the record. Back when I was working on Redmine's API I was planning to just use a simple Hash for this and change to_xml and to_json to use this Hash for building the string. It's not completely thought out yet but a rough code example would be like this:

 1class Issue < ActiveRecord::Base
 2
 3  # This is the standard method we would maintain
 4  def api_format(options={})
 5    {
 6      :subject => subject,
 7      :description => description,
 8      :some_array => [1,2,3]
 9    }
10  end
11
12  # These would be standardized and probably just need to wrap Rails to use our api_format method
13  #
14  # They could even be put into a common module and added to the Models or ActiveRecord itself.
15
16  def to_json(options, &block)
17    # ... send JsonSerializer the results from api_format... TODO
18  end
19
20  def to_xml(options, &block)
21    # ... send XmlSearializer the results from api_format... TODO
22  end
23end
24
25# Plugin
26module IssuePatch
27  # ... normal patching
28
29  # Example of how a plugin can patch the method and add a new field to the API
30  def api_format_with_new_field(options={})
31    core_api_data = api_format_without_new_field(options)
32    core_api_data[:new_field] = 'some value the plugin adds'
33    core_api_data
34  end
35end

Feedback

What does everyone think? Should we stick with Redmine's API? Should we take the time now to build an API that will be easier to maintain?

Eric Davis

P.S. compatibility between ChiliProject 1.x and 2.x shouldn't be a problem because we are allowed to break backwards compatibility but we should commit to an API format from 2.x on so others can integrate with ChiliProject. I'm not as concerned with keeping API compatibility with Redmine, we can mimic it's public API or fake it out. We also don't know the long terms plans for Redmine's API.


Replies (15)

RE: Proposal - REST API change - Added by Enrique García Cota at 2011-02-11 08:50 am

I'm not really familiar with the current Redmine interface.

Is it a read-only? If it's used for any POST requests, wouldn't this also need a create_from_xml and update_from_xml or something similar, in order to be functionally equivalent?

Appart from that, it seems more rails-oriented, and more maintainable, so I like it.

RE: Proposal - REST API change - Added by Eric Davis at 2011-02-11 07:41 pm

Enrique García Cota wrote:

Is it a read-only? If it's used for any POST requests, wouldn't this also need a create_from_xml and update_from_xml or something similar, in order to be functionally equivalent?

No it's both read and write. Rails converts the XML/JSON data into params which we can use to create/update a record in the controller (not shown). I think the current "incoming" part of the REST API is fine, it's the "outgoing" response generation I don't like.

Eric Davis

RE: Proposal - REST API change - Added by Felix Schäfer at 2011-02-14 07:24 am

Enrique García Cota wrote:

Appart from that, it seems more rails-oriented, and more maintainable, so I like it.

Seconded.

RE: Proposal - REST API change - Added by Jeffrey Hulten at 2011-02-15 04:59 pm

Maintainable and extendable are the key requirements from my perspective. My only issue with the 'Rails way' is the .xml .json etc... For my money a great API would let us go to https://www.chiliproject.org/issues/189 with a request header of Accept: application/json or Accept: text/xml to get the other types. One URI to reference one resource. This also opens the door to some awesome RDFa doors in the future.

RE: Proposal - REST API change - Added by Michael Richardson at 2011-03-24 06:27 pm

Jeffrey Hulten wrote:

For my money a great API would let us go to https://www.chiliproject.org/issues/189 with a request header of Accept: application/json or Accept: text/xml to get the other types. One URI to reference one resource. This also opens the door to some awesome RDFa doors in the future.

My understanding of rails 1.2 and 2.x (I'm so far ignorant of 3.x) is that if you put in an Accept: header, that it sets the format value, and it works exactly as you say, but that few web browsers actually let you do that, thus the need for the extensions.

RE: Proposal - REST API change - Added by Jeffrey Hulten at 2011-03-25 07:55 pm

Michael Richardson wrote:

My understanding of rails 1.2 and 2.x (I'm so far ignorant of 3.x) is that if you put in an Accept: header, that it sets the format value, and it works exactly as you say, but that few web browsers actually let you do that, thus the need for the extensions.

How often do browers use the API? I think jQuery sets the Accept header, which is the only browser case I can think of.

RE: Proposal - REST API change - Added by Michael Richardson at 2011-03-28 07:51 am

Jeffrey Hulten wrote:

How often do browers use the API? I think jQuery sets the Accept header, which is the only browser case I can think of.

That's my point: the extensions were there for the rare case when a developer or broken-web-client user wanted to test something,
that rails stack expects Accept: to be set.

RE: Proposal - REST API change - Added by Peer Allan at 2011-04-11 12:19 pm

Have you considered creating a versioned API by segregating it into its own section?

  http://chilliproject.com/api_v1/
  http://chilliproject.com/api_v2/

It can cause some duplication of controller logic, but it allows much better reliability for users of the API. Without something like this user are going to have to constantly update their client libs to match the API changes for every project update. One of my biggest complaints about APIs is when they change right under me.

Peer

RE: [Proposal] - REST API change - Added by Chris Woerle at 2011-06-15 12:21 pm

HI Edavis, thanks for this elaboration.

I will have to work with the API and the current API doesn't my needs, so I am looking for an easy way to change it.

My two cents:

1) I agree with Peer Allan totally, that we should version the api if any possible, this will lower the overhead.
Can someone write the code to do that easily?

2) We are working in our team with InheritedResources these days a lot. It allowes doing this:

# subclass or mixin inherited resource
class IssuesController < ...
  respond_to :xml
end

This would work, If what edavis suggested, @issue.to_xml would work.

3. Builder templates
My experience with @something.to_xml was -> nice, no problem.
But @something.to_csv -> not good, because: I wanted to customize a lot for doing 'csv export'.

Conclusion: It might be really helpful, to fallback to templates in case. Inherited Resource will fallback to to_xml if no template exists.

This leads to -> Plugins can just add their own templates with no need to override to_xml.

4. I am looking for a hammer method, that keeps my coordination overhead low. That could be this.


class Issue < ..
  # if this is not sufficcient, roll our own
  accepts_nested_attributes_for \All Resources\
end

class ApiController < InheritedResources::Base
  respond_to :xml, :json, :csv, :pdf
  before_filter :validate_api_access
  private
  def validate_api_access
    # ... just stub
  end
end

class Api::IssuesController < ApiController
  # this maps 'projects/:id/issues/:id'
  belongs_to :project
end

5) The Model should receive a mixin like edavis promoted.
Things a cool api should provide include 'partial doing things', basically trigger SELECT commands in order to customize the fields returned and :include options in order to return relations also.

I am not quite sure, how this would integrate easily in the promoted api_format - method

RE: [Proposal] - REST API change - Added by Michael Richardson at 2011-06-15 06:56 pm

Chris Woerle wrote:

2) We are working in our team with InheritedResources these days a lot. It allowes doing this:

I'm a bit concerned that this might make too many things implicit, and I don't know
how this is going to work with the desire to have API calls added by plugins.
I think that the controllers do not follow a sufficiently regular pattern that this is worth it,
but I'm willing to be proven wrong.

This would work, If what edavis suggested, @issue.to_xml would work.

3. Builder templates
My experience with @something.to_xml was -> nice, no problem.
But @something.to_csv -> not good, because: I wanted to customize a lot for doing 'csv export'.

It's not just that, what is returned depends upon the permissions of the user logged
in. RSS and API are two formats with API keys, and I actually want to expose a bit less
to via the API than to the human.... in my plugin!

What do I do right now?
I think that I should just add new stanza to the respond_to, and perhaps rip out the .api
each time. I have no idea how we are going to build a plugin mechanism that lets a plugin
do this.

RE: [Proposal] - REST API change - Added by Chris Woerle at 2011-06-16 08:33 am

Michael Richardson wrote:

Chris Woerle wrote:

2) We are working in our team with InheritedResources these days a lot. It allowes doing this:

I'm a bit concerned that this might make too many things implicit, and I don't know
how this is going to work with the desire to have API calls added by plugins.
I think that the controllers do not follow a sufficiently regular pattern that this is worth it,
but I'm willing to be proven wrong.

I think you are right, though the controllers went a long way to become more - lets say 'resourcy'.

That's why I'd rather do something else, like the other approach with an ApiController

It's not just that, what is returned depends upon the permissions of the user logged
in. RSS and API are two formats with API keys, and I actually want to expose a bit less
to via the API than to the human.... in my plugin!

Yes, I will review in the next hours, how the permissions could be used the same way, they are right now.

Regarding Plugins registering API calls...
Good point, I think I ignored that a bit.

If a plugin is following nested resources ideas, then, let's say you'd have tags, then you'd just allow a route 'issues/:id/tags'.
That would work. But if you'd want to add fancy API methods, I have no clue.

Do you think of adding new formats? That would work.
Do you think of adding totally new methods? Then they would be either other controllers or while breaking the CRUD pattern add a new action including the route to the existing controller right?
That would also work.

Also I could imagine something like this (not as a before_filter actually):

before_filter :handle_api_hooks

def handle_api_hooks
  self.class.api_hooks.each_pair{|p,hook| hook.call(params) if p == [params[:action],params[:format]]}
end

This would allow customizing

What do I do right now?
I think that I should just add new stanza to the respond_to, and perhaps rip out the .api
each time. I have no idea how we are going to build a plugin mechanism that lets a plugin
do this.

I agree, this won't be possible.

What about this:

Instead of

respond_to do |format|
 ...
end

we call

handle_api_hooks

directly.

This way we'd have to customize the responders per each action like this:


def self.api_hooks
  @api_hooks ||= { 
   ["index","csv"] => lambda {|p| Issue.all_to_csv }
 }
end

def self.api_hooks= arg
  @api_hooks = arg
end

Are going to work through all the controllers soon?

RE: [Proposal] - REST API change - Added by Chris Woerle at 2011-06-16 11:42 am

OK, reading all the code, I think I will just write a plugin, that adds api behaviour to the controllers I need and keep the same form like it is now.

Here http://praktikanten.brueckenschlaeger.org/2011/06/16/documenting-the-redmine-api
is a documentation, that I hope is useful for people who want to understand the api system.

Can offer Textile for the wiki.

I think the best, that plugins can do for now, is to just override the builder-templates and exclude all the stuff you don't want to expose to the api.

RE: [Proposal] - REST API change - Added by Eric Davis at 2011-06-17 04:00 pm

Chris Woerle wrote:

1) I agree with Peer Allan totally, that we should version the api if any possible, this will lower the overhead.
Can someone write the code to do that easily?

In an ideal world I would agree. Having a versioned API would make things easier for clients.

But in reality that would take up a lot of time and effort to maintain. For example, in 2.0.0 we are changing a lot how Journals work so if we had a versioned API we would have to:

  1. Branch off the previous behavior (call it /api/v1)
  2. Add the new Journals code
  3. Create a new API for the new Journals behavior (/api/v2)
  4. Figure out how to connect the old (v1) API to the new data. For Journals that is hard but I can think of a few changes that would be impossible to maintain backwards compatibility (e.g. removing or renaming major data)

This is why we committed to using semver with our releases. We will maintain that the API won't change (much) during from 1.x release to 1.x release. During the 1.x release to the 2.x release we are allowed to break the API (and any backwards compatibility).

When someone wants to help out to maintain the API and also add versioning, I'd be totally happy to do it then. I can only speak for my own time and as it it, I'm limited in what I can accomplish.

P.S. I don't want to use InheritedResources for this. It does too much automatically and we need more control over what data is exposed. I use it in my plugins and have added some plugin APIs that way, but I don't think it's a good fit for here.

Eric Davis

RE: [Proposal] - REST API change - Added by Peer Allan at 2011-09-23 09:52 pm

While I understand the logic and DRY-ness of using the built in Rails methods it does not appeal to me. I have attempted to use the built in methods, it starts out simple then gets messy very quickly when trying to expand what you want returned in the JSON/XML. If you want a nice example I recommend reading this article, http://engineering.gomiso.com/2011/05/16/if-youre-using-to_json-youre-doing-it-wrong/. Another piece that isn't mentioned in the article is cascading into associated objects. If you load an Issue and ask it to include its journals (:include => :journals in the to_json call), Rails converts the journals to JSON, but if you defined a custom to_json on Journal it will not be called. (I will admit I have not checked this recently so if it has changed I would be happy to know)

The reference article talks about "The Ruby API Builder Language" or RABL (if not familiar take a look here, https://github.com/nesquena/rabl). Add has some nice code examples on how to use RABL to generate you API templates and still be DRY. In addition the templates are extendable which could be useful for plugin authors. The part I like best, is that since they are normal templates you can include authorization logic and that allows you to do things like show and hide fields based on authorization or permission levels. For example, an admin logging in can get the email addresses of all the users in the journals, but a normal user just gets the names. This is virtually impossible using the built in Rails methods.

http://engineering.gomiso.com/2011/05/16/if-youre-using-to_json-youre-doing-it-wrong/
http://engineering.gomiso.com/2011/06/27/building-a-platform-api-on-rails/
http://seesparkbox.com/foundry/better_rails_apis_with_rabl

Another factor that may want to be considered is how REST-like you want the API to be. I think for the most part that Eric's plans address that, but I thought I would throw in a couple of articles that inspired my team to give everyone a nice perspective on REST when thinking about how to develop the API. There is also an alternative solution to API versioning described in the first article that is much cleaner than the approach I asked about earlier.

http://blog.steveklabnik.com/2011/07/03/nobody-understands-rest-or-http.html
http://blog.steveklabnik.com/2011/08/07/some-people-understand-rest-and-http.html

To summarize, I think RABL based templates are the way to go to build the API.

If I can get our Redmine instance switched over to CP soon, then I will need an expanded API and would like to take crack at it.

Thanks for great work everyone!

RE: [Proposal] - REST API change - Added by Felix Schäfer at 2011-10-02 05:33 pm

Peer Allan wrote:

While I understand the logic and DRY-ness of using the built in Rails methods it does not appeal to me.

I don't know exactly how far down the road we are, but I tend to agree with this, and this seems to be a growing gripe with the Rails core with a new plugin/gem scratching that itch being announced every other week. I'm not sure either that having what we want to pass the API should be in the models themselves, either split that into APIFormatters or (even better) find a good way to use templates but still be configurable enough (e.g. want to add an attribute to the API? Just add a partial _my_plugin.api_extension and be done with it. That wouldn't solve the problem of removing attributes from the API, but would we want to enable that?).

The reference article talks about "The Ruby API Builder Language" or RABL (if not familiar take a look here, https://github.com/nesquena/rabl). Add has some nice code examples on how to use RABL to generate you API templates and still be DRY. In addition the templates are extendable which could be useful for plugin authors. The part I like best, is that since they are normal templates you can include authorization logic and that allows you to do things like show and hide fields based on authorization or permission levels.

Sounds nice. I had a quick look at the github page and was somewhat puzzled by the alias format (:attribute => :alias? I'd probably have done it the other way round…), but other than that it looks like a usable starting point.

Another factor that may want to be considered is how REST-like you want the API to be. I think for the most part that Eric's plans address that, but I thought I would throw in a couple of articles that inspired my team to give everyone a nice perspective on REST when thinking about how to develop the API. There is also an alternative solution to API versioning described in the first article that is much cleaner than the approach I asked about earlier.

I'd also be in favor of header-based API versioning rather than path/URI based. Are there any known drawbacks to header-based ones, e.g. some often-used "API client libraries" not being able to set it, or is it just laziness/crooked convention holding things (generally, not just for CP) back?

(1-15/15)