rest-gen: Review ruby client generation

[bergmark]

The status of it is unknown to me, as a first step we should generate the ruby client for rest-example and make sure it works properly.

[DNNX]

Hi,

A bit of feedback on the generated file from my side, hope you'll find it useful. I've pasted the ruby client there. Here are my notes. Mostly stylistic issues, but anyway.

• It is not valid ruby :) ruby -c rest-example-generated-client.rb (-c stands for check syntax only) claims that there's syntax error on line 166. The reason is that on line 96 two lines of code concatenated together. Just grep for endrequire 'net/http'.
• def method (args) must be def method(args)
• A lot of redundant return statements. return x.to_s should be x.to_s and [pdat, res] is more natural than return pdat, res.
• People usually omit brackets when defining a method with no arguments. I.e. def x, not def x().
• Traditionally methods are named using snake_case rather than camelCase or PascalCase.
• Usually there is a blank line between methods.

I haven't tried to run the client yet, will let you know if any issue arises.

BTW, how can I build happstack example app? When I do cabal run rest-example-happstack it says this:

cabal: Cannot build the executable 'rest-example-happstack' because the
component is marked as disabled in the .cabal file.

I can see that there is the happstack flag in the .cabal file, but how can I turn it on? I tried to google it, found this, but still I have no idea how to use these flags. I ended up changing if flag(happstack) to if True in the rest-example.cabal file, but I'm pretty sure there must be more appropriate way to run rest-example on happstack.

[bergmark]

Thanks a lot for looking into this!

The syntax errors are obviously bad :) and converting to snake_case also sounds like a good change. The stylistic things you mention also seem reasonable, I wouldn't consider them a high priority since you don't edit these files but I certainly wouldn't mind the changes.

For the haskell generation (and hopefully soon for js generation) we took the step to create an AST instead of using code-builder since it's easier to manage. Is there a haskell library for pretty printing ruby from an AST? This is also just a nice-to-have.

You need to compile rest-example with -fhappstack (or snap/wai) to get an api running executable, otherwise it just builds the library and the generate executable.

[DNNX]

Ok, I've inserted the newline between end and require on the line 96 and run ruby -r./rest-example-generated-client. The file doesn't load, giving me the following error:

Fix url encoding when this is going to be used/Users/viktar/Dropbox/work/tmp/rest/rest-example/rest-example-generated-client.rb:152:in `<module:SilkApi>': uninitialized constant SilkApi::BaseApi (NameError)
    from /Users/viktar/work/tmp/rest/rest-example/rest-example-generated-client.rb:151:in `<top (required)>'
    from /Users/viktar/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:45:in `require'
    from /Users/viktar/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/rubygems/core_ext/kernel_require.rb:45:in `require'

BaseApi class is actually in RestexampleApi module (lines 1 and 3), but it is expected to be either in global namespace (i.e ::BaseApi) or in SilkApi module (i.e. ::SilkApi::BaseApi). It's on the line 50. I'll fix the issue manually for now and see whether the client works with my amendments.

Thanks for the -fhappstack hint BTW! Works like a charm.

As of pretty-printing haskell libraries for ruby AST - unfortunately I haven't seen any. But my experience in Haskell is quite limited, maybe there exist some libs I'm not aware of.

[DNNX]

And it worked! At least Post.list. Here's one-liner to test the client:

$ ruby -r ./rest-example-generated-client.rb -e "p SilkApi::Api.new('http://localhost:3000').Post.list"

Fix url encoding when this is going to be used[{"offset"=>0, "count"=>2, "items"=>[{"createdTime"=>"2014-04-01T13:37:00.000Z", "content"=>"Just wanted to tell the world!", "author"=>"erik", "id"=>1, "title"=>"Rest is awesome"}, {"createdTime"=>"2014-03-31T15:34:00.000Z", "content"=>"Hello world!", "author"=>"adam", "id"=>0, "title"=>"First post"}]}, #<Net::HTTPOK 200 OK readbody=true>]

To recap, I did two changes in the generated file.

1. Line break in line 96.
2. class Api < BaseApi -> class Api < RestexampleApi::BaseApi, line 152.

UPD: For the record, I ran all my tests against https://github.com/silkapp/rest/commit/ec8a05fc0c06bf12de39cd42cdda25857ba4595b .

[bergmark]

Cool, big thanks!

• We could create a ruby script based on script.sh that runs those requests as tests and then make travis run that.
• We also need to update the code generator to fix the changes you listed

@DNNX would you be interested in doing any of this?

[DNNX]

@bergmark , I've already fixed the generator, the PR will be ready within an hour. As of the script.sh thing - I'll maybe do it later, but I don't promise :) Not today definitely.

[bergmark]

To summarize here, we now generate valid ruby code at least :-) Here are some remaining things to do:

• The ruby template has some references to Silk, they shouldn't be there, SilkApi should be renamed and the login method should be removed completely.
• We need to handle cookies, is there anything shipped with ruby that provides a cookie jar?
• We're now just reading in the template file (files/Ruby/base.rb) and doing some textual replacement in it to get the correct names. This is pretty hacky and it'd be better to either build an AST or just inline those parts in the code using code-builder.
• Write a script using the ruby client that we can run on travis as regression tests.

[nh2]

I think the get code is wrong:

def get (params = {}, headers = {})
  internalSilkRequest(@api, :get, @url, params, 'text/plain', @dataType, headers)

This is 7 arguments when internalSilkRequest takes 8.

I suspect there should be an extra nil

def get (params = {}, headers = {})
  internalSilkRequest(@api, :get, @url, params, 'text/plain', @dataType, nil, headers)

like in the generated list call:

def list (params = {}, headers = {})
  internalSilkRequest(@api, :get, @url + '', params, 'text/plain', :json, nil, headers)

Also not sure why this one does @url + '' when the other one doesn't.