Update dredd to support nullable attribute #283
Comments
|
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. |
|
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. |
|
Unfortunately, we can't announce nullable support until apiaryio/drafter#121 is done. But other than that, we have a bit of work currently happening after which there will be a new drafter release. |
|
@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)? |
|
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? |
|
Dredd uses drafter.js which uses protagonist 0.x. We need to move dredd to protagonist 1.x. |
|
That makes perfect sense, thanks. I'm kind of afraid to ask but what is waiting on moving dredd to protagonist 1.x? Thanks. |
|
I think @netmilk will be more suited to answer this. |
|
Sounds good, thanks. |
|
@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. |
|
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 |
|
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
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. |
|
I think this is more of a correct outline.
|
|
@pksunkara thanks. What is involved in #1 and how can I assist? |
|
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:
passIt still fails with the error You can find the offending code at https://github.com/mitodl/ccxcon/tree/project-skeleton The relevant files are |
|
@justinabrahms There is no keyword called "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"
}
}
} |
|
Just wanted to update that this did in fact solve my issue. Thank you, @kylef! |
|
Hey whats happening here? I've got a field Any ideas? |
|
@philsturgeon Dredd hasn't updated to the latest protagonist yet. |
|
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. |
|
You can use nullable, but you have to do it in your dredd testcase. It looks something like this: mitodl/ccxcon@a999c12#diff-e2c3cc6906fca70b577ef650a4fdcf54R29 |
|
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? |
|
@philsturgeon @netmilk can give you a better update than me. /cc @zdne |
|
@philsturgeon Yes it is ... without fixing the JSON Schema it won't work. |
Nullable Attribute MSON type workaroundHere's a little workaround. If you use these hooks given bellow and include a 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 #nullableJSON 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 |
|
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. |
|
Haha ouch: |
|
@philsturgeon Tanks for the feedback! I'll check it. |
|
Excellent news! Thanks for the updates :) Phil Sturgeon
|
|
Well installing via source has not gone well. I tried a few things, including AKA old as f**k. I guess I'll just wait for the tagged version?
|
|
@philsturgeon, I updated the JS hooks above. Please try it again. I should work™. Please note this workaround will work only with 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. |
|
Once #338 is merged, |
|
@honzajavorek This should be closed, right? |
|
Yes, nullable should work. |
|
I know that Swagger doesn't support |
|
@nicolasiensen Swagger is just JSON Schema, so, you can write something similar for nullable. |
|
@nicolasiensen did you get |
|
Update: Got it to work with: http://stackoverflow.com/questions/38139657/nullable-fields-in-swagger-on-node-js/38140597#38140597 |
Oh I really really wish this was true, but it isn't. Unlike @fungjj92 suggested, swagger-api/swagger-editor#1302 Open API 3.0 supports Can we get some support for this? |
|
@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. |
|
I wrote an essay. apiaryio/fury-adapter-swagger#112 |
|
For those who just want there tests to work when a response returns id:
type: integer
description: The ID of something.
minimum: 0To the following. id:
type:
- integer
- 'null'
description: The ID of something.
minimum: 0Your Dredd tests should now accept EDIT: Don't do this although it passes validation its technically not valid and breaks tools like The |
|
@synthecypher Hi! Have you found it? Regarding
I'm struggling with the same issue currently |
|
Since the fury-adapter-swagger issue got solved (apiaryio/fury-adapter-swagger#112) when can we expect this to be available in Dredd? |
|
#993 will make this available to Dredd. |
|
I must be missing something, I
But I still get an error:
This is my swagger definition for the property: I thought this would work but did I forget or misunderstood something? |
|
@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 apiaryio/fury-adapter-swagger#172, hopefully we can get this reviewed tomorrow and released into production/Dredd. |
|
Ah ok that explains it, thanks for the quick response and work. I will await the fix, let me know if I can be of any help. |
|
@InfopactMLoos Dredd 5.1.7 should include a fix for your problem. |
Dredd should properly test against the new
nullableattribute 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.
The text was updated successfully, but these errors were encountered: