Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dredd ignores Request Parameters #395

Closed
Alexorz opened this issue Mar 4, 2016 · 19 comments
Closed

Dredd ignores Request Parameters #395

Alexorz opened this issue Mar 4, 2016 · 19 comments

Comments

@Alexorz
Copy link

Alexorz commented Mar 4, 2016

Hi there,

I got a trouble when running dredd (v1.0.5), and it seems to be an issue, here is my apib file:

## Coupons Collection [/api/coupons{?pn,pb}]

### List All Coupons [GET]

+ Parameters
    + pn: 1 (optional) - Page number
    + pb: 20 (optional) - Page by
    + blablabla...

+ Response 200 (application/json)
    + Attributes
        + blablabla...


### Create a New Coupon [POST]
> when I run `dredd` for this .apib file,
this request will be ignored if I did not assign any parameters here,
but actually there is no need to pass any parameters in this request.

+ Request (application/json)
    + Attributes
        + title: `another coupon` (string, required) - title of the coupon
        + blablabla...

+ Response 201 (application/json)
    + Attributes
        + blablabla...

Here is the result:

complete: 1 passing, 0 failing, 0 errors, 0 skipped, 1 total

The POST request was been ignored when running dredd, because I didn't assign any parameters for this. But actually there is no need to pass any URL parameters for this POST.

So, am I doing right?

@Alexorz Alexorz changed the title test case been ignored when without assigning parameters test case be ignored when without assigning parameters Mar 4, 2016
@honzajavorek
Copy link
Contributor

@Alexorz Thanks for the example! I think the problem might be that your + Parameters section is nested under the GET specifically, so in the context of POST Dredd can't have an idea what pn and pb are and whether they're optional or not.

If you move the + Parameters one level higher, it should work (worked for me when I tried to reproduce your problems):

## Coupons Collection [/api/coupons{?pn,pb}]

+ Parameters
    + pn: 1 (optional) - Page number
    + pb: 20 (optional) - Page by

### List All Coupons [GET]

+ Response 200 (application/json)
    + Attributes
        + something

### Create a New Coupon [POST]

+ Request (application/json)
    + Attributes
        + title: `another coupon` (string, required) - title of the coupon
        + something

+ Response 201 (application/json)
    + Attributes
        + something

Is this solution of the issue?

@honzajavorek
Copy link
Contributor

@Alexorz Feel free to re-open this issue if you get back to this and will think that my answer isn't solution to your problem. As of now I consider this as solved.

@Alexorz
Copy link
Author

Alexorz commented Mar 17, 2016

Hello @honzajavorek , the dredd testing works fine in your way. But as a document, I think it's more reasonable to put + Parameters under each request, because there always be some parameters which only works for single request type. Just like the pn and pb is only needed in GET request in this case.

And I saw apiary.io has already read the .apib in the way that I want:

FORMAT: 1A
HOST: http://polls.apiblueprint.org/

# test

## Coupons Collection [/api/coupons{?pn,pb}]

### List All Coupons [GET]

+ Parameters
    + pn: 1 (optional) - Page number
    + pb: 20 (optional) - Page by

+ Response 200 (application/json)


### Create a New Coupon [POST]

+ Request (application/json)
    + Attributes
        + title: `another coupon` (string, required) - title of the coupon

+ Response 201 (application/json)

qq20160317-3 2x
qq20160317-4 2x

@honzajavorek honzajavorek reopened this Mar 17, 2016
@honzajavorek
Copy link
Contributor

@Alexorz Thanks for pointing this out! If Apiary picks your markup as you expect, then there's an inconsistency and I should look into it - if it's not a bug, then at least to find out reasoning behind it so I'm able to answer here.

Possibly related issues: apiaryio/api-blueprint#277, apiaryio/api-blueprint#58 (Note for myself, I need to verify this.)

I'll try to get to this soon and verify the behavior, but I can't promise you any ETA now. However, thank you again for sharing this. This may be a valid bug report either for Dredd or Apiary!

@kylef
Copy link
Member

kylef commented Mar 18, 2016

Future versions of API Blueprint allow you to attach parameters to requests, as per RFC 4: Request Parameters. The last pre release of Drafter and Protagonist provide a partial implementation of this feature, however semantic validation around these parameters is not yet complete.

I'd say this isn't a bug in either Dredd or Apiary, they are just lacking this feature. I believe #388 is around adding support for this in Dredd.

@honzajavorek
Copy link
Contributor

@kylef Yes, thanks for providing all the details! I thought this is the case and that was my reasoning why Dredd doesn't work well with this and the API Blueprint needs to be written in a different way. However, as @Alexorz explained on screenshots, Apiary is somehow able to support his syntax. And that's what surprised me and I wanted to explore that a bit deeper and learn how is that possible.

@kylef
Copy link
Member

kylef commented Mar 18, 2016

@honzajavorek From the screenshot, you can see this document is being rendered with the attributes kit feature flag, which means it's using the latest pre-release of the parser which adds partial support for parsing these parameters. However they are not rendered in Apiary documentation or utilised in the Apiary mock server since they haven't adopted this feature yet.

@Alexorz I'd recommend you stick to using the resource or action level parameters until we've finalised the support otherwise the information will be missing in your documentation at Apiary and not utilised in Dredd.

@honzajavorek
Copy link
Contributor

document is being rendered with the attributes kit feature flag

Ah! I didn't realise that. Thanks for spotting, now that all makes perfect sense.

@Alexorz Apiary with the attributes kit feature flag turned on is a bit ahead than Dredd and you've ran into a beta feature being under development. @kylef thanks again for bringing light into this.

I'm turning this issue into a request for improvement - we should eventually catch up with RFC 4: Request Parameters and make sure it's properly supported in Dredd once it's fully present in Drafter.

@honzajavorek honzajavorek changed the title test case be ignored when without assigning parameters Dredd ignores Request Parameters Mar 18, 2016
@Alexorz
Copy link
Author

Alexorz commented Mar 19, 2016

Cool! Looking forward to a better dredd and API Blueprint.
Thank you all :)

@joaosa
Copy link

joaosa commented Aug 16, 2016

Any updates on this? :)

@kylef
Copy link
Member

kylef commented Aug 16, 2016

I think this was actually supported since Dredd 1.1.0. I tested this in apiaryio/api-blueprint#58 (comment), where you can find an example.

@honzajavorek
Copy link
Contributor

honzajavorek commented Aug 16, 2016

I can confirm it is experimentally supported since Drafter supports it to some extent. Not sure how complete the implementation in Drafter is at the moment.

Dredd should be ready to inherit the parameters correctly, but there might be other little things needed to be done. The feature wasn't ever discussed yet in its entirety as a Dredd feature, with all the corner cases. Also it is not tested nor documented yet.

@honzajavorek
Copy link
Contributor

Just a reminder, once this is going to be closed, #88 should be closed, too.

@honzajavorek
Copy link
Contributor

@kylef What's the status of request parameters in Drafter? Is it finalized?

@kylef
Copy link
Member

kylef commented Mar 8, 2017

@honzajavorek It's supported, but validation is not implemented fully. Nothing has changed since apiaryio/api-blueprint#58 (comment)

@gustaff-weldon
Copy link

gustaff-weldon commented Jan 12, 2018

I'm still having this issue. POST is ignored if parameters are only specified for GET. As for OP they only apply to GET and does not make sense for POST.

Below is an example spec that can be used to reproduce:

FORMAT: 1A

# Example API

The API for the EatBacon IOT project

## Gists Collection [/gists{?since}]
Collection of all Gists.

### List All Gists [GET]
+ Parameters
    + since (optional, string) ... Timestamp in ISO 8601 format: `YYYY-MM-DDTHH:MM:SSZ` Only gists updated at or after this time are returned.

+ Response 200

### Create a Gist [POST]
To create a new Gist simply provide a JSON hash of the *description* and *content* attributes for the new Gist. 

+ Request (application/json)
    + Attributes
        + descripton: `sth sth` (string, required)
        + content: `here goess content` (string, required)

+ Response 201 (application/json)

Drakov runs the server and picks up two routes:

   DRAKOV STARTED
[LOG] Setup Route: GET /gists List All Gists
[LOG] Setup Route: POST /gists Create a Gist
   Drakov 1.0.4      Listening on port 3000

But Dredd, complains about ambiguous parameter and completely ignores POST:

info: Configuration './dredd.yml' found, ignoring other arguments.
warn: Compilation warning in file 'apiary.apib': Ambiguous URI parameter in template: /gists{?since}
Parameter not defined in API description document: since ( > Gists Collection > Create a Gist)
info: Beginning Dredd testing...
pass: GET (200) /gists duration: 40ms
complete: 1 passing, 0 failing, 0 errors, 0 skipped, 1 total
complete: Tests took 893ms

Apiary editor picks parameters just fine only for GET.

@honzajavorek @kylef any suggestions other than moving parameters one level up (which I'd like to avoid since it makes API spec less clear)

@kylef
Copy link
Member

kylef commented Jan 12, 2018

@gustaff-weldon How about the following:

## Gists Collection [/gists]

### List All Gists [GET /gists{?since}]

+ Parameters
    + since: `2016-12-30T10:00:00Z` (string, optional) - Timestamp in ISO 8601 format: `YYYY-MM-DDTHH:MM:SSZ` Only gists updated at or after this time are returned.

+ Response 204

### Create a Gist [POST]

+ Request (application/json)
    + Attributes
        + descripton: `sth sth` (string, required)
        + content: `here goess content` (string, required)

+ Response 201 (application/json)

@gustaff-weldon
Copy link

@kylef cool, I think moving parameter down to GET, will work for us. Thanks!
Btw, apiary.io was fine with a previous version, is that a feature of Dredd or is it apiary.io that uses older parser/validator?

@honzajavorek
Copy link
Contributor

Even though it's not an officially rolled-out feature in API Blueprint and Apiary, the parser supports request parameters for a long time already and Dredd does as well. Please open other issues if you find the behavior buggy. Closing this in favor of #1118.

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

No branches or pull requests

5 participants