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

Adding swagger #1

Merged
merged 9 commits into from Jul 10, 2013

Conversation

Projects
None yet
2 participants

No description provided.

@cwalsh cwalsh and 1 other commented on an outdated diff Jul 9, 2013

lib/api_canon/documented_action.rb
@@ -36,6 +48,7 @@ def initialize(action_name)
# ```
#
def param(param_name, options={})
+ options = {:http_method => http_method}.merge options
@cwalsh

cwalsh Jul 9, 2013

Owner

Shouldn't this be only on the documented_action rather than on each documented_param for that action? I can see you need to use it later to work out the param type (form, query, url etc.), so we probably need to provide a reference on the documented_param back to the documented_action to delegate this to.

The line below could become something like

@params[param_name] = DocumentedParam.new param_name, self, options

Then DocumentedParam would delegate http_method to that second parameter, e.g.

class DocumentedParam
  #...more code
  attr_accessor :documented_action
  def initialize(name, documented_action, opts={})
    @name = name
    @documented_action = documented_action
    opts.each {|k,v| self.send("#{k}=", v) }
  end

Anyway, talk more tomorrow.

@leondewey

leondewey Jul 9, 2013

Yeah thats much better!

@cwalsh cwalsh commented on the diff Jul 9, 2013

lib/api_canon/swagger/base.rb
@@ -0,0 +1,27 @@
+module ApiCanon
+ module Swagger
+ class Base < ActiveModel::Serializer
+
+ self.root = false
+ attributes :api_version => :apiVersion
+ attributes :swagger_version => :swaggerVersion
+ attributes :base_path => :basePath
+ attributes :apis
+
+ def api_version
@cwalsh

cwalsh Jul 9, 2013

Owner

Hmmm, tricky. I could add this as a parameter to DocumentedController, so users would type:

class FooController
  document_controller version: '3'
  # Or perhaps
  document_controller do
    version 3
  end
end

This does of course require people to have a different controller for each API version, but that doesn't seem too arduous a requirement for now.

Owner

cwalsh commented Jul 9, 2013

Looks great! Tomorrow I'll have a look at adding parent references to the DocumentedParam and DocumentedAction classes, should make some things a bit easier.

⌥  rake
/Users/ldewey/.rvm/rubies/ruby-2.0.0-p0/bin/ruby -S rspec ./spec/lib/api_canon/documented_action_spec.rb ./spec/lib/api_canon/documented_param_spec.rb ./spec/lib/api_canon/swagger_spec/api_declaration_spec.rb ./spec/lib/api_canon/swagger_spec/resource_listing_spec.rb ./spec/lib/api_canon_spec.rb
...*..................

Pending:
  ApiCanon::DocumentedAction response_code describes the response codes the user can expect to see
    # Need to create objects to deal with this similarly to DocumentedParam
    # ./spec/lib/api_canon/documented_action_spec.rb:22

Finished in 0.01208 seconds
22 examples, 0 failures, 1 pending

Here is some example output:

/swagger-doc
screen shot 2013-07-10 at 10 25 44 pm

/swagger-doc/streams
screen shot 2013-07-10 at 10 26 11 pm

@leondewey leondewey commented on the diff Jul 10, 2013

lib/api_canon/swagger/api_declaration.rb
+ :action => object.action_name,
+ :only_path => true
+ }
+
+ object.params.keys.each do |name|
+ url_params[name] = '{name}' unless name == :format
+ end
+
+ url = URI.unescape url_for(url_params)
+
+ # This is required because we dont know if the params are
+ # path params or query params, this way we dont care.
+ url = url.split('?').first
+
+ "#{url}.{format}"
+ end
@leondewey

leondewey Jul 10, 2013

@cwalsh I have fixed/cleaned this up... It should work for any path params now. Only thing is that you need to have the params documented. Other wise it will error, do you think thats ok or should it just rescues and say just put a error in the path field. Personally i kind of like the real error.

@cwalsh

cwalsh Jul 10, 2013

Owner

It is possible to ask the application routes for the significant keys (path params), but this is fine for now.

@leondewey

leondewey Jul 10, 2013

Ohh that is what I was wanting! do you know how? I did a quick pry last night and did not see it, this is how its done on the grape side... then some gsubs to change the params to "{name}"

@cwalsh

cwalsh Jul 11, 2013

Owner

Some of the following would be useful:

  def routes
    Rails.version.starts_with?('2') ? ActionController::Routing::Routes.routes : Rails.application.routes.routes
  end

  def significant_keys(route)
    Rails.version.starts_with?('2') ? route.significant_keys : route.segment_keys
  end

@leondewey leondewey commented on the diff Jul 10, 2013

lib/api_canon/swagger/api_declaration.rb
+ def path
+ url_params = {
+ :controller => object.controller_name,
+ :action => object.action_name,
+ :only_path => true
+ }
+
+ object.params.keys.each do |name|
+ url_params[name] = '{name}' unless name == :format
+ end
+
+ url = URI.unescape url_for(url_params)
+
+ # This is required because we dont know if the params are
+ # path params or query params, this way we dont care.
+ url = url.split('?').first
@leondewey

leondewey Jul 10, 2013

This a bit hacky, but could not think of a better way

maybe this but its not any simpler

uri = URI url_for(url_params)
"#{uri.host}://#{uri.path}.{format}"

@cwalsh I think its good to go... you?

This is a little "crazy" but since there could be more than one version when looking at a group of controllers

@cwalsh cwalsh added a commit that referenced this pull request Jul 10, 2013

@cwalsh cwalsh Merge pull request #1 from leondewey/master
Adding swagger
15bc47f

@cwalsh cwalsh merged commit 15bc47f into cwalsh:master Jul 10, 2013

1 check failed

default The Travis CI build could not complete due to an error
Details
Owner

cwalsh commented Jul 10, 2013

Probably more important to get Swagger in at all, and then figure out the niceties later. Merged.

Owner

cwalsh commented Jul 10, 2013

Turns out it doesn't work so well in Rails 3. At least, the tests don't. https://travis-ci.org/cwalsh/api_canon/builds/8923816

Owner

cwalsh commented Jul 10, 2013

Okay, fixed the Rails 3 issues, couple more with Ruby 1.8 https://travis-ci.org/cwalsh/api_canon/builds/8924804 but I'll have a look at those tomorrow.

Thanks and sorry.

We need it to work in rails 2 as well right?

Might need to re-think this:

In Gemfile:
  combustion (>= 0) ruby depends on
    rails (>= 3.1.0.rc5) ruby

@cwalsh cwalsh added a commit that referenced this pull request Jul 12, 2013

@cwalsh cwalsh Merge pull request #1 from leondewey/master
Gem dep fix
81ec23f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment