Update dredd to support nullable attribute

[obihann]

Dredd should properly test against the new nullable attribute supported by MSON and drafter.

md + s1 (string, nullable)

json {"s1":null}

I can help with this however I am not 100% sure if I need to make changes to drafter.js or protagonist?

Thanks.

[netmilk]

Hi @obihann,

you're right, this a Drafter issue. Drafter compiles JSON schema from MSON and this JSON schema is used for Gavel validation of server response. Since this feature will be introduced in Drafter it's only a matter of bumping Drafter version in Dredd to support it.

[obihann]

Awesome! @pksunkara when can can we expect a new release of drafter? I know nullable is in the master branch since I was so involved in that, but it seems that we need a new tag release so that Dredd can point specifically to that.

[pksunkara]

Unfortunately, we can't announce nullable support until https://github.com/apiaryio/drafter/issues/121 is done. But other than that, we have a bit of work currently happening after which there will be a new drafter release.

[obihann]

@pksunkara thanks, I'll try to take a look at that tonight and help you guys out a bit.

@netmilk I just want to verify all that is holding this up is in fact apiaryio/drafter, I ask this because I understood dredd uses apiaryio/drafter.js (which is being replaced by apiaryio/protagonist)?

[obihann]

I think I have my confusion figured out @netmilk... Dredd uses drafter.js which is a wrapper (?) for protagonist which uses drafter via a git module. Right?

[pksunkara]

Dredd uses drafter.js which uses protagonist 0.x. We need to move dredd to protagonist 1.x.

[obihann]

That makes perfect sense, thanks. I'm kind of afraid to ask but what is waiting on moving dredd to protagonist 1.x?

Thanks.

[pksunkara]

I think @netmilk will be more suited to answer this.

[obihann]

Sounds good, thanks.

[obihann]

@netmilk I'm running into some difficulty right now testing an update to dredd so that it will use drafter.js and the latest (currently master branch) version of drafter. I have a feeling this may be because (as @pksunkara) dredd needs to use protagonist 1.x directly and eliminate the reference to drafter.js.

My question is how complex or simple do you think that will be, and what are the blockers preventing it from happening?

I have a high priority issue right now with the project I am on that we cannot properly test our API with dredd due to the null limitations, I've been working closly with @pksunkara to implement this on MSON, snowcrash, and drafter, now that that is close to complete dredd is the next focus of my attention. I can help out on this, I just need to know what the blockers are so I can tackle them.

[netmilk]

Hi @obihann,

Mainly, thank you very much for all your enthusiasm and and contributions but at this moment, updating to the latest protagonist won't solve your issue with nullable parameter. Drafter (not .js) used in the latest protagonist doesn't have feature parity with drafter.js (yes, .js) which supports JSON schema rendering from MSONs. Updating to the protagonist would cause complete disabling of validation of MSON structures in Dredd.

@zdne confirmed the Blueprint team is working on support of JSON schema rendering in the Drafter (not .js) used in the latest protagonist and they are treating it as a priority.

If this is a limitation for using Dredd in your actual project, I would propose some workaround in beforeAll hooks where you can modify the generated schema in the transaction object under the expected.body.schema property.

[obihann]

Hi @netmilk, thank you for your reply. Yes, this may be a limitation, so I will have to look into your suggested work around.

Regarding protagonist, drafter, and drafter.js... Let me see if I fully understand this.

Dredd (latest release) uses Drafter.js (latest release), which uses Protagonist (older 0.x release), which finally uses Dredd (older release again).

The latest version of Protagonist (1.x) uses a newer version of Dredd (not the latest though), however this results in a different feature set than what Dredd.js (using 0.x Protagonist) provides? Because of this my attempts to migrate Dredd to Protagonist 1.x and eliminate Drafter.js is actually a very large task that will not solve my original issues.

Adding all that together, I am seeing that the latest version of Drafter does have the features I need, and the scheduled 1.1.0 release of Drafter will also include 'nullable'. So this is a rough outline of what has to happen before Dredd can properly support nullable

1) 1.1.0 release of Drafter 2) Protagonist updated to 1.1.0 version of drafter 3) Dredd updated to use the above mentioned release of Protagonist

Does all that sound about right? Sorry, if I am way off.

If I am actually correct, then I think I need to work with @zdne currently to get the 1.1.0 release of Drafter taken care of, and then Protagonist updated to use that. Then I can return to Dredd.

[pksunkara]

I think this is more of a correct outline.

1) JSON Schema rendering support in Drafter 2) 1.1.0 release of Drafter 3) Protagonist updated to 1.1.0 version of drafter 4) Dredd updated to use the above mentioned release of Protagonist

[obihann]

@pksunkara thanks. What is involved in #1 and how can I assist?

[justinabrahms]

I'm having some difficulty with the workaround. Can someone validate that this is roughly what you guys were expecting?

@hooks.before_all
def make_moudle_price_nullable(transactions):
    for t in transactions:
        try:
            schema = json.loads(t['expected']['bodySchema'])
            schema['nullable'] = ['price_per_seat_cents']
            t['expected']['bodySchema'] = json.dumps(schema)
        except KeyError:
            pass

It still fails with the error fail: body: At '/price_per_seat_cents' Invalid type: null (expected number). I'm using dredd @ 1.0.1. I've also tried making expected.bodySchema.price_per_seate_cents.type = 'number,nullable' and it didn't seem to work.

You can find the offending code at https://github.com/mitodl/ccxcon/tree/project-skeleton The relevant files are apiary.apib and apiary_hooks.py in the root of the repository. Full failure below.

Configuration dredd.yml found, ignoring other arguments.
info: Using apiary reporter.
warn: Apiary reporter environment variable APIARY_API_KEY or APIARY_API_NAME not defined.
info: Beginning Dredd testing...
Spawning `python` hooks handler
Hook handler stdout: Starting Dredd Python hooks handler

Hook handler stderr: apiary_hooks.py:1: RuntimeWarning: Parent module 'apiary_hooks' not found while handling absolute import
  import json
apiary_hooks.py:2: RuntimeWarning: Parent module 'apiary_hooks' not found while handling absolute import
  import os
apiary_hooks.py:4: RuntimeWarning: Parent module 'apiary_hooks' not found while handling absolute import
  import django
apiary_hooks.py:5: RuntimeWarning: Parent module 'apiary_hooks' not found while handling absolute import
  import dredd_hooks as hooks
apiary_hooks.py:10: RuntimeWarning: Parent module 'apiary_hooks' not found while handling absolute import
  from courses.models import Course, Module

pass: GET /api/v1/coursexs/ duration: NaNms
pass: POST /api/v1/coursexs/ duration: NaNms
pass: GET /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ duration: NaNms
pass: DELETE /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ duration: NaNms
skip: PATCH /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/
pass: PUT /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ duration: NaNms
pass: GET /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/ duration: NaNms
fail: POST /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/ duration: 57ms
skip: GET /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1db1a8439cbf4a8f8c395ad63fb43e8a/
skip: DELETE /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1db1a8439cbf4a8f8c395ad63fb43e8a/
skip: PATCH /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1db1a8439cbf4a8f8c395ad63fb43e8a/
skip: PUT /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1db1a8439cbf4a8f8c395ad63fb43e8a/
skip: GET /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ccxs
skip: POST /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ccxs
skip: GET /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ccxs/bcfa43b4
skip: DELETE /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ccxs/bcfa43b4
skip: PATCH /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ccxs/bcfa43b4
skip: PUT /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/ccxs/bcfa43b4
skip: GET /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules
skip: POST /api/v1/ccxs/7e0e52d0386411df81ce001b631bdd31/modules/bcfa43b4
skip: DELETE /api/v1/ccxs/7e0e52d0386411df81ce001b631bdd31/modules/bcfa43b4
info: Displaying failed tests...
fail: POST /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/ duration: 57ms
fail: body: At '/price_per_seat_cents' Invalid type: null (expected number)

request: 
body: 
{
  "title": "Introduction to Quantum Bits",
  "subchapters": [
    "Quarks and Similar things",
    "Things that are not quarks"
  ]
}
headers: 
    Content-Length: 140
    Content-Type: application/json
    User-Agent: Dredd/1.0.1 (Linux 3.19.0-23-generic; x64)

uri: /api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/
method: POST

expected: 
body: 
{
  "uuid": "1140283aa1b24f5e895e97a76a339040",
  "title": "Introduction to Quantum Bits",
  "price_per_seat_cents": null,
  "subchapters": [],
  "course": "http://localhost:8000/api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/",
  "url": "/api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1140283aa1b24f5e895e97a76a339040/"
}
headers: 
    Content-Type: application/json
    Location: /coursexs/7e0e52d0386411df81ce001b631bdd31/modules/{uuid}/

bodySchema: {"$schema": "http://json-schema.org/draft-04/schema#", "required": ["price_per_seat_cents"], "type": "object", "properties": {"uuid": {"type": "string"}, "title": {"type": "string"}, "url": {"type": "string", "description": "url of the module"}, "price_per_seat_cents": {"type": "number", "description": "price in cents to avoid poor decimal support"}, "course": {"type": "string", "description": "url of the parent course"}, "subchapters": {"type": "array"}}, "nullable": ["price_per_seat_cents"]}
statusCode: 201

actual: 
body: 
{
  "uuid": "1143d755c5e547d39a715c17460ddd76",
  "title": "Introduction to Quantum Bits",
  "subchapters": [
    "Quarks and Similar things",
    "Things that are not quarks"
  ],
  "course": "http://localhost:8000/api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/",
  "price_per_seat_cents": null,
  "url": "/api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1143d755c5e547d39a715c17460ddd76/"
}
headers: 
    vary: Accept, Cookie
    server: WSGIServer/0.1 Python/2.7.9
    location: http://localhost:8000/api/v1/coursexs/7e0e52d0386411df81ce001b631bdd31/modules/1143d755c5e547d39a715c17460ddd76/
    allow: GET, POST, HEAD, OPTIONS
    date: Fri, 06 Nov 2015 19:43:01 GMT
    x-frame-options: SAMEORIGIN
    content-type: application/json

statusCode: 201

complete: 6 passing, 1 failing, 0 errors, 14 skipped, 21 total
complete: Tests took 5098ms
Hook handler closed with status: null
complete: See results in Apiary at: https://app.apiary.io/public/tests/run/89718e6a-bf63-450f-8875-f18c009be39e

[kylef]

@justinabrahms There is no keyword called nullable in JSON Schema, you will need to add null to the list of types for the property you want to allow to be null.

 "price_per_seat_cents": {
-  "type": "number",
+  "type": ["number", "null"],
   "description": "price in cents to avoid poor decimal support"

Original JSON Schema:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
   "required": [
    "price_per_seat_cents"
  ],
   "type": "object",
   "properties": {
    "uuid": {
      "type": "string"
    },
     "title": {
      "type": "string"
    },
     "url": {
      "type": "string",
       "description": "url of the module"
    },
     "price_per_seat_cents": {
      "type": "number",
       "description": "price in cents to avoid poor decimal support"
    },
     "course": {
      "type": "string",
       "description": "url of the parent course"
    },
     "subchapters": {
      "type": "array"
    }
  }
}

New JSON Schema:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
   "required": [
    "price_per_seat_cents"
  ],
   "type": "object",
   "properties": {
    "uuid": {
      "type": "string"
    },
     "title": {
      "type": "string"
    },
     "url": {
      "type": "string",
       "description": "url of the module"
    },
     "price_per_seat_cents": {
      "type": ["number", "null"],
       "description": "price in cents to avoid poor decimal support"
    },
     "course": {
      "type": "string",
       "description": "url of the parent course"
    },
     "subchapters": {
      "type": "array"
    }
  }
}

[justinabrahms]

Just wanted to update that this did in fact solve my issue. Thank you, @kylef!

[philsturgeon]

Hey whats happening here? I've got a field - destination (object, nullable, optional) and it's giving the error: fail: body: At '/trips/destination' Invalid type: null (expected object).

Any ideas?

[pksunkara]

@philsturgeon Dredd hasn't updated to the latest protagonist yet.

[philsturgeon]

Ahh, so im currently trying to get my API docs back up to date and using dredd to do this, should I just give up? Is there a workaround? I'll need to make a LOT of custom nonsense MSON data structures if I can't use nullable.

[justinabrahms]

You can use nullable, but you have to do it in your dredd testcase. It looks something like this:

https://github.com/mitodl/ccxcon/commit/a999c12d80e5cbc3e3dd00140aa2f0c3ad2edae5#diff-e2c3cc6906fca70b577ef650a4fdcf54R29

[philsturgeon]

I see comments in here talking about that work around fixing the JSON Schema, but I'm not concerned about that, I just want dredd to work. Is that one and the same?

[pksunkara]

@philsturgeon @netmilk can give you a better update than me. /cc @zdne

[w-vi]

@philsturgeon Yes it is ... without fixing the JSON Schema it won't work.

[netmilk]

Nullable Attribute MSON type workaround

Here's a little workaround. If you use these hooks given bellow and include a #nullable hashtag in the Attribute description, the null will be added as an acceptable type to the JSON Schema. The only catch is the #nullable hashtag will be rendered in the documentation.

cc @justinabrahms @philsturgeon @obihann

Example blueprint:

# My API

# My Resource [/my_resource]

# Its Action [GET]

+ Response 200 (application/json)

  + Attributes (My Structure)

# Data Structures

## My Structure (object)

- myKey: myValue (string)
- destination:  (object, nullable, optional) - Some description #nullable
- someObject: (object)
  - itsKey: (string, nullable, optional) - Some other description #nullable

JSON Schema before:

{
  "type": "object",
  "properties": {
    "myKey": {
      "type": "string"
    },
    "destination": {
      "type": "object",
      "description": "Some description #nullable"
    },
    "someObject": {
      "type": "object",
      "properties": {
        "itsKey": {
          "type": "string",
          "description": "Some other description #nullable"
        }
      }
    }
  },
  "$schema": "http://json-schema.org/draft-04/schema#"
}

JSON Schema after:

{
  "type":"object",
  "properties":{
    "myKey":{
      "type":"string"
    },
    "destination":{
      "type":[
        "object",
        "null"
      ],
      "description":"Some description #nullable"
    },
    "someObject":{
      "type":"object",
      "properties":{
        "itsKey":{
          "type":[
            "string",
            "null"
          ],
          "description":"Some other description #nullable"
        }
      }
    }
  },
  "$schema":"http://json-schema.org/draft-04/schema#"
}

JavaScript hook:

var hooks = require('hooks');

// Recursively add null as acceptable type if there is string
// "#nullable" present in the property description
var patchPropertiesWithNullable = function(schema) {
  if (typeof(schema['properties']) == 'object' && ! Array.isArray(schema['properties'])){
    for (property in schema['properties']){
      var partialSchemaToPatch = schema['properties'][property];
      schema['properties'][property] = patchPropertiesWithNullable(partialSchemaToPatch);
    };
  };

  if (schema['description'] !== undefined) {
    if (schema['description'].indexOf("#nullable") > -1) {
      if (schema['type'] === undefined) {
        schema['type'] = 'null';

      } else if (typeof(schema['type']) == 'string') {
        schema['type'] = [schema['type'], 'null'];

      } else if (Array.isArray(schema['type'])) {
        schema['type'].push('null');

      };
    };
  };

  return(schema);
};

hooks.beforeAll(function(transactions, callback) {
  for(index in transactions) {
    var transaction = transactions[index];
    if(transaction['expected']['bodySchema'] !== undefined){
      var schema = JSON.parse(transaction['expected']['bodySchema']);
      schema = patchPropertiesWithNullable(schema);
      transactions[index]['expected']['bodySchema'] = JSON.stringify(schema, null, 2);
    };
  };

  callback();
});

Ruby hook port:

require 'json'
include DreddHooks::Methods

# Recursively add null as acceptable type if there is string
# "#nullable" present in the property description
def patch_properties_with_nullable schema
  if !schema['description'].nil?
    if schema['description'].include? "#nullable"
      if schema['type'].nil?
        schema['type'] = 'null'
      elsif schema['type'].is_a? String
        schema['type'] = [schema['type'], 'null']
      elsif schema['type'].is_a? Array
        schema['type'].push 'null'
      end
    end
  end

  if schema['properties'].is_a? Hash
    schema['properties'].each do |key, value|
      schema['properties'][key] = patch_properties_with_nullable value
    end
  end
  schema
end

before_all do |transactions|
  transactions.each_with_index do |transaction, index|
    if ! transaction['expected']['bodySchema'].nil?
      schema = JSON.parse transaction['expected']['bodySchema']
      schema = patch_properties_with_nullable schema
      transactions[index]['expected']['bodySchema'] = schema.to_json
    end
  end
end

[philsturgeon]

I added some logging and my hooks are definitely running (using your JS example) but it's just not adding null to the JSON schema. Debugging where exactly it's going wrong.

[philsturgeon]

Haha ouch:

error: undefined duration: NaNms
error: TypeError: Cannot read property 'description' of undefined
  at patchPropertiesWithNullable (/Users/phil/src/foo-app/hooks.js:7:13)
  at patchPropertiesWithNullable (/Users/phil/src/foo-app/hooks.js:26:43)
  at /Users/phil/src/foo-app/hooks.js:37:16
  at TransactionRunner.runLegacyHook (/opt/boxen/nodenv/versions/v0.12.9/lib/node_modules/dredd/lib/transaction-runner.js:240:9)

[netmilk]

@philsturgeon Tanks for the feedback! I'll check it.

[philsturgeon]

Any luck on this @netmilk?

[kylef]

@philsturgeon I just want to point out, support for nullability was added to dredd in master (release pending). So you should be able to build Dredd from source and test out nullability without these workarounds.

[philsturgeon]

Excellent news! Thanks for the updates :)

Phil Sturgeon Sent from my iPhone and there's probably typos because I'm probably at the pub.

On Dec 16, 2015, at 5:07 PM, Kyle Fuller notifications@github.com wrote:

@philsturgeon I just want to point out, support for nullability was added to dredd in master (release pending). So you should be able to build Dredd from source and test out nullability without these workarounds.

— Reply to this email directly or view it on GitHub.

[philsturgeon]

Well installing via source has not gone well. I tried a few things, including npm install -g git+ssh@github.com:apiaryio/dredd.git and it is missing the lib folder which causes some issues. I tried fixing that running ./scripts/build but it doesnt do anything, aaaand a git log inside the installed dredd folder is:

commit f851881958aa0e17c4f942fe4b5419e6de7c9ab5
Author: cronopio <aristizabal.daniel@gmail.com>
Date:   Sat Jul 18 23:49:29 2015 -0500

    :ship: Bump version v0.3.4

AKA old as f**k.

I guess I'll just wait for the tagged version?

[netmilk]

@philsturgeon, I updated the JS hooks above. Please try it again. I should work™. Please note this workaround will work only with npm install -g dredd@1.0.1. Ping me on Twitter DM in case it won't work and you need any mental support. We can schedule a little Skype or whatever else. I'm happy to help.

And I have bad news regarding support of nullable without this dirty hack. Last release of Protagonist and last release Dredd enabled support for Node 4 and Node 5, but unfortunately it still doesn't support nullable in JSON schemas. Many apologies for misleading information.

[pksunkara]

Last release of Protagonist and last release Dredd enabled support for Node 4 and Node 5, but unfortunately it still doesn't support nullable in JSON schemas

Ouch. We need to fix that soon. /cc @w-vi @kylef

[honzajavorek]

Once https://github.com/apiaryio/dredd/pull/338 is merged, nullable should be supported in Dredd. Although the original goal of the Pull Request is slightly different, it contains basic integration test also for nullable and it seems to work correctly.

[pksunkara]

@honzajavorek This should be closed, right?

[honzajavorek]

Yes, nullable should work.

[nicolasiensen]

I know that Swagger doesn't support nullable yet, but is there any workaround to define nullable properties in Swagger in a way that Dredd can resolve?

[pksunkara]

@nicolasiensen Swagger is just JSON Schema, so, you can write something similar for nullable.

[fungjj92]

@nicolasiensen did you get null to work for Swagger? Running into it now

[fungjj92]

Update: Got it to work with: type: [string, 'null']

http://stackoverflow.com/questions/38139657/nullable-fields-in-swagger-on-node-js/38140597#38140597

[philsturgeon]

Swagger is just JSON Schema,

Oh I really really wish this was true, but it isn't.

Unlike @fungjj92 suggested, type: [string, 'null'] is not valid Swagger (even though it is valid JSON Schema).

https://github.com/swagger-api/swagger-editor/issues/1302

Open API 3.0 supports nullable: true, which ReDoc also backported so is what a lot of folks use in v2.0. It's what im using too.

Can we get some support for this?

[honzajavorek]

@philsturgeon Would you mind to raise the question in https://github.com/apiaryio/fury-adapter-swagger/issues/ ? If it's becoming a de-facto standard even for 2.0, I quite like the idea.

[philsturgeon]

I wrote an essay. https://github.com/apiaryio/fury-adapter-swagger/issues/112

[jshbrntt]

For those who just want there tests to work when a response returns null for a field just change the type in the specification from.

      id:
        type: integer
        description: The ID of something.
        minimum: 0

To the following.

      id:
        type:
          - integer
          - 'null'
        description: The ID of something.
        minimum: 0

Your Dredd tests should now accept null for the field.

EDIT: Don't do this although it passes validation its technically not valid and breaks tools like swagger-codegen

The x-nullable extension attribute is the way to go, I just need to find I way to make the tests pass in the same way it does when defining type: ['integer', null].

[scythargon]

@synthecypher Hi! Have you found it? Regarding

The x-nullable extension attribute is the way to go, I just need to find I way to make the tests pass in the same way it does when defining type: ['integer', null].

I'm struggling with the same issue currently

[InfopactMLoos]

Since the fury-adapter-swagger issue got solved (https://github.com/apiaryio/fury-adapter-swagger/issues/112) when can we expect this to be available in Dredd?

[kylef]

https://github.com/apiaryio/dredd/pull/993 will make this available to Dredd.

[InfopactMLoos]

I must be missing something, I

• updated to dredd 5.1.6,
• added the x-nullable attribute with value true

But I still get an error:

body: At '/customer_reference' Invalid type: null (expected string)

This is my swagger definition for the property:

"customer_reference": {
  "type": "string",
  "description": "(Optional) reference tag for customer",
  "x-nullable": true
}

I thought this would work but did I forget or misunderstood something?

[kylef]

@InfopactMLoos Looks like you're hitting a bug because x-nullable is not being applied recursively (only at root of the schema). I've fixed this in https://github.com/apiaryio/fury-adapter-swagger/pull/172, hopefully we can get this reviewed tomorrow and released into production/Dredd.