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

Already on GitHub? Sign in to your account

Gem api enhancements #336

Merged
merged 4 commits into from Aug 22, 2011

Conversation

Projects
None yet
2 participants
Contributor

markmcspadden commented Aug 19, 2011

I wanted to expose the "New Gems" and "Just Updated" section that appears on the homepage as part of the api. (I looked and couldn't find it...if it's there and I missed it....my bad.) To do so I added Api::V1::RubygemsController#latest and Api::V1::RubygemsController#just_updated.

They are currently modeled after Api::V1::DownloadsController#top for better or for worse. In that model, each pulls 50 entries, which is probably excessive. In fact, in this process, Version#just_updated was edited to allow for a custom limit. The two new methods also do NOT require authentication just like Api::V1::DownloadsController#top.

Tests are there, again modeled after Api::V1::DownloadsController#top.

Let me know if you'd like to see any modifications or think this is just a bad idea in general. Thanks! (Super cool that you can fork the source of Rubygems.org and play with it...much props.)

Mark

Member

sferik commented Aug 19, 2011

This patch looks good to me and supports the principles laid out in https://github.com/rubygems/rubygems.org/wiki/API-v2 (specifically, number 4).

However, before I merge this change, I'd like to see a corresponding patch that adds documentation for these new APIs to the guides.

It would also be nice if you added support for these new resources to gems, however that will not block the merging of this patch.

@markmcspadden are you willing to make these additional changes?

@sferik sferik commented on an outdated diff Aug 19, 2011

app/controllers/api/v1/rubygems_controller.rb
@@ -55,6 +55,28 @@ class Api::V1::RubygemsController < Api::BaseController
render :text => "The version #{params[:version]} is already indexed.", :status => :unprocessable_entity
end
end
+
+ def latest
+ object = {
+ :gems => Rubygem.latest(50).map { |rubygem| rubygem.attributes }
+ }
+ respond_to do |format|
+ format.json { render :json => object }
+ format.xml { render :xml => object }
+ format.yaml { render :text => object.to_yaml }
@sferik

sferik Aug 19, 2011

Member

One minor note: in the other YAML responses, we convert the objects to JSON before converting them to YAML in order to strip the object types (e.g. !ruby/ActiveRecord:Rubygem) from the response. See lines 15-17 and 26-28 of this file. We should probably be doing the same thing here, if only for consistency.

If you can think of a more elegant way to strip the object types, I'd encourage you to implement that instead, replacing this admittedly brutish approach.

I should add, this step isn't necessary in Ruby 1.9.3, which uses psych as the default yamler. Perhaps we can get around this by using the psych gem, however, I've run into some issues with getting it to load before syck.

@sferik sferik commented on an outdated diff Aug 19, 2011

app/controllers/api/v1/rubygems_controller.rb
+ }
+ respond_to do |format|
+ format.json { render :json => object }
+ format.xml { render :xml => object }
+ format.yaml { render :text => object.to_yaml }
+ end
+ end
+
+ def just_updated
+ object = {
+ :gems => Version.just_updated(50).map {|version| version.attributes}
+ }
+ respond_to do |format|
+ format.json { render :json => object }
+ format.xml { render :xml => object }
+ format.yaml { render :text => object.to_yaml }
@sferik

sferik Aug 19, 2011

Member

Ditto. See note on line 66.

Contributor

markmcspadden commented Aug 19, 2011

I'm on board with the changes. May be this evening or tomorrow but I'm good for them. Thanks for the feedback.

Clean up Api::V1::RubygemsController #latest and #just_updated, inclu…
…ding the perfered YAML process of converting to JSON first.
Contributor

markmcspadden commented Aug 22, 2011

I think I've got it all together now.

Latest commit does the JSON-first YAMLizing.

Pull request submitted to guides regarding the new methods: rubygems/guides#9

Let me know if there's anything I've missed.

sferik added a commit that referenced this pull request Aug 22, 2011

@sferik sferik merged commit 083c79e into rubygems:master Aug 22, 2011

@sferik sferik referenced this pull request Feb 15, 2012

Closed

API route collision #401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment