riboseinc / ribose-api

API for Ribose
0 stars 0 forks source link

The request/response sample #19

Open abunashir opened 6 years ago

abunashir commented 6 years ago

I am going through the document and I think we are using "" string to represent some context, but I was thinking should we use some fake content like for name it could beJohn Doe or something? Then we can also use that mock server response to stub the API responses in the client library?

Samples: https://github.com/riboseinc/ribose-ruby/tree/master/spec/fixtures

ribose-jeffreylau commented 6 years ago

Good idea.

I'm just not so sure how to do it with the MSON we have. Maybe it's just a simple matter of adding default values to the response attributes?

abunashir commented 6 years ago

@ribose-jeffreylau: I assume you already figured it out? Do you need a helping hand here?

ribose-jeffreylau commented 6 years ago

@abunashir Yes, I have been adding example values as I go along.

What would be great is if we could import the fixtures from ribose-ruby directly as the example responses.

I'm not entirely sure if examples responses can co-exist with MSON specifications, though. Worth a try.

skalee commented 6 years ago

I'm not entirely sure if examples responses can co-exist with MSON specifications, though. Worth a try.

I did that in #43, validator says it's okay.

And I totally agree that importing fixtures from the other project will make things easier to work on. For instance, sections/space.apib is dominated by duplicated response samples.

We can either use git submodules, or simply clone things in a makefile prior apiary.apib compilation. I guess I'll experiment a bit.

ribose-jeffreylau commented 6 years ago

@skalee

So, if we go for the Git submodule route, I'd imagine that the fixtures are transcluded into the Body sections individually?

skalee commented 6 years ago

@ribose-jeffreylau Precisely, I want it to look like in this gist: https://gist.github.com/skalee/b9d0d9e70e73f15cf7fed815da8e5cb5

ribose-jeffreylau commented 6 years ago

@skalee Looks good!

skalee commented 6 years ago

A possible disadvantage of submodules is that they need to be updated manually. Alternatively, we could grab content straight from GitHub via HTTP. Hercule provides a remote content resolver out of the box which we can reuse in our custom fixture resolver. It's seems quite decent, even does some caching.

I believe that it is beneficial to keep a reference to ribose-ruby's branch, commit or version tag. Submodules obviously provide that. With HTTP requests, we need to find a place to store this information, however such data can be kept in package.json.

So, your choice:

  1. Git submodules, which require some manual work,
  2. downloading fixtures from GitHub, master branch,
  3. downloading fixtures from GitHub, branch pointed in package.json.
ronaldtse commented 6 years ago

I think submodules provide exactly the behavior we need except that we need to update it. However, the manual update is beneficial that we can be certain what exactly is included in the documentation. To supplement this we can have a Travis build that checks if the git submodule needs updating?

skalee commented 6 years ago

Sure we can enhance our Travis checks. It's just 1) update submodules 2) rebuild apiary.apib 3) check if git status lists it as modified.

ronaldtse commented 6 years ago

Exactly — let’s do it. Thanks @skalee!

skalee commented 6 years ago

Actually, I got another question. I've noticed that some JSONs in the APIBs are indented with four spaces whereas the others with two spaces. All JSONs in ribose-ruby project fixtures are indented with two spaces. Which style should be used in APIB? My personal preference goes to two spaces as they can be transcluded without re-indenting.

cc: @ronaldtse @ribose-jeffreylau

ronaldtse commented 6 years ago

I fully agree with JSON indented with two spaces!

skalee commented 6 years ago

If anyone wonders how to transclude fixtures, take #59 as an example. Of course you need to set up a git submodule first which is not described in README yet:

git submodule init
git submodule update
skalee commented 6 years ago

I started replacing hard-coded response examples with fixtures transcluded from ribose-ruby. Unfortunately, there are issues. Response fixtures are often different from what has been written in APIBs quite recently. For instance, response example for wiki creation action in wiki.apib:

https://github.com/riboseinc/ribose-api/blob/646eea1924ea2d56b4bfa765619c3e800cbf3e1b/sections/wiki.apib#L166-L247

is visibly different from wiki.json fixture in ribose-ruby (e.g. the latter doesn't have tags object):

https://github.com/riboseinc/ribose-ruby/blob/6f6289c3ebb8380da4ff3efbf49f07f6fc372568/spec/fixtures/wiki.json

Furthermore, the wiki page from ribose-ruby fixtures has different attributes set which doesn't correspond to request parameters at all. Not to mention it's revision 23, which looks unsuitable for a freshly created wiki page.

I am starting to realize that response examples should be generated from API server test suite. For instance, test which covers wiki page creation action should export to some file request parameters and recorded response, so that it can be used both as ribose-ruby fixture and example response for API blueprints.

ronaldtse commented 6 years ago

@skalee I agree that better examples will be needed. Would you be able to help clean it up on both ribose-ruby and the blueprint sides?

skalee commented 6 years ago

I guess I could update response fixtures, but IMO there'll be constant maintenance problem with them. Even if API won't change frequently, it will be easy to forget about them.

I think the best option is to generate them automatically, perhaps from your server's test suite. If we are going to maintain them manually, then some tool to prove their correctness would be welcome. However, I suppose that autogenerating them from your main project's test suite will be the easiest to do.

abunashir commented 6 years ago

Hi, I think quite late here, sorry about that but somehow I overlooked notifications from this conversation, but here are some of the thoughts and couple of potential improvements we might consider in the future.

The first issue I can see is with the fixtures file in ribose-ruby repo. Those do not exactly represent the complete responses for any of the API calls, I did that way to keep those minimal, and just to ensure that the parsing of the ribose response work regardless of every single attribute. So, if we directly use those on the response sample then that might not exactly represent the response.

Another thing, I think the git submodule should the way around, have the fixtures files in the ribose-api repo with actual response and then maybe submodule those from the ribose-ruby repo or any other API client we will have in the future.

That way we don't have to update an API client every time we have an update in our response fixtures, and if we want to take it further then we can also consider having a completely separate repo only with the api-response and then submodule those from any of the repo dependent on those.

cc: @ronaldtse, @ribose-jeffreylau , @skalee

ribose-jeffreylau commented 6 years ago

So we have two courses of action (which seem to be complementary rather than mutually exclusive):

  1. Auto-generating API responses from main project's test suite.

  2. Keep the generated responses in ribose-api, or even a separate API rep ribose-api-response, such that client projects like ribose-ruby could include it as a Git submodule. (And ribose-api could submodule ribose-api-response.)

Any thoughts?

abunashir commented 6 years ago

@ribose-jeffreylau: Do we already have any project/test suite that goes through every single endpoint and we can easily export the responses?

skalee commented 6 years ago

IMO it's much easier and simpler to keep recorded fixtures in a separate repository. In this approach we don't need to care much about history prettiness, because all contributors are bots anyway. We can push updates as often as we want without polluting ribose-api history, which should stay human-readable for sure. Furthermore, all merge conflicts can be resolved by overwriting old stuff.