Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Support top level Reference Element when translating API Elements to JSON & JSON Schema #710

Merged
merged 3 commits into from
May 31, 2019
+609 −22
Merged

Support top level Reference Element when translating API Elements to JSON & JSON Schema #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e71cc6
fix(json-schema): support top level reference element
tjanc May 31, 2019
180d8d9
fix(json-value): support top level reference element
tjanc May 31, 2019
ea42d1e
chore: add changelog entry
tjanc May 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Master

### Bug Fixes

* JSON value & JSON schema backends now support top-level Reference Elements.
This prevents crashes on `Include` statements in `enum` contexts.
[#709](https://github.com/apiaryio/drafter/issues/709)

### Enhancements

* The default build type is now release when using cmake.
Expand Down
46 changes: 26 additions & 20 deletions src/refract/JsonSchema.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,28 +323,29 @@ namespace
void renderProperty(ObjectSchema& s, const ExtendElement& e, TypeAttributes options);
void renderProperty(ObjectSchema& s, const IElement& e, TypeAttributes options);

so::Object& renderSchema(so::Object& schema, const ObjectElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const ArrayElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const EnumElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const NullElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const StringElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const NumberElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const BooleanElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const ExtendElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const ObjectElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const ArrayElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const EnumElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const NullElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const StringElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const NumberElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const BooleanElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const ExtendElement& e, TypeAttributes options);
so::Object& renderSchemaSpecific(so::Object& schema, const RefElement& e, TypeAttributes options);
so::Object& renderSchema(so::Object& schema, const IElement& e, TypeAttributes options);

template <typename T>
void renderProperty(ObjectSchema&, const T& e, so::Object&, TypeAttributes)
{
LOG(error) << "invalid property element: " << e.element();
assert(false);
abort();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the rationale behind changing assert to abort please, not sure I understand the reason

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we want to avoid continuing the program flow because we should never arrive there. Specifically, bug #709 caused a SIGSEGV by overflowing the stack through unchecked recursion - with an abort here, there would be the much nicer SIGABORT.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, perhaps it would be better to avoid exiting the process though? Perhaps throwing std::exception would be better, the drafter_parse/validate function could catch and return appropriate error to caller?

}

template <typename T>
so::Object& renderSchema(so::Object& s, so::Object&, const T& e, TypeAttributes)
so::Object& renderSchemaSpecific(so::Object& s, const T& e, TypeAttributes)
{
LOG(error) << "invalid top level element: " << e.element();
assert(false);
abort();
return s;
}

Expand All @@ -355,7 +356,7 @@ namespace
template <typename ElementT>
void operator()(const ElementT& el)
{
renderSchema(*schemaPtr, el, options);
renderSchemaSpecific(*schemaPtr, el, options);
}
};

Expand Down Expand Up @@ -389,8 +390,13 @@ so::Object schema::generateJsonSchema(const IElement& el)

namespace
{
so::Object& renderSchemaSpecific(so::Object& s, const RefElement& e, TypeAttributes options)
{
const auto& resolved = resolve(e);
return renderSchema(s, resolved, passFlags(options));
}

so::Object& renderSchema(so::Object& s, const ObjectElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& s, const ObjectElement& e, TypeAttributes options)
{
constexpr const char* TYPE_NAME = "object";

Expand Down Expand Up @@ -421,7 +427,7 @@ namespace
return schema;
}

so::Object& renderSchema(so::Object& s, const ArrayElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& s, const ArrayElement& e, TypeAttributes options)
{
constexpr const char* TYPE_NAME = "array";

Expand Down Expand Up @@ -463,7 +469,7 @@ namespace
return s;
}

so::Object& renderSchema(so::Object& schema, const EnumElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& schema, const EnumElement& e, TypeAttributes options)
{
options = updateTypeAttributes(e, options);

Expand Down Expand Up @@ -524,7 +530,7 @@ namespace
return schema;
}

so::Object& renderSchema(so::Object& schema, const NullElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& schema, const NullElement& e, TypeAttributes options)
{
addType(schema, "null");
return schema;
Expand Down Expand Up @@ -573,22 +579,22 @@ namespace
return s;
}

so::Object& renderSchema(so::Object& s, const StringElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& s, const StringElement& e, TypeAttributes options)
{
return renderSchemaPrimitive(s, e, options);
}

so::Object& renderSchema(so::Object& s, const NumberElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& s, const NumberElement& e, TypeAttributes options)
{
return renderSchemaPrimitive(s, e, options);
}

so::Object& renderSchema(so::Object& s, const BooleanElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& s, const BooleanElement& e, TypeAttributes options)
{
return renderSchemaPrimitive(s, e, options);
}

so::Object& renderSchema(so::Object& s, const ExtendElement& e, TypeAttributes options)
so::Object& renderSchemaSpecific(so::Object& s, const ExtendElement& e, TypeAttributes options)
{
auto merged = e.get().merge();
renderSchema(s, *merged, options);
Expand Down
11 changes: 9 additions & 2 deletions src/refract/JsonValue.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,14 @@ namespace
so::Value renderValueSpecific(const NumberElement& element, TypeAttributes options);
so::Value renderValueSpecific(const BooleanElement& element, TypeAttributes options);
so::Value renderValueSpecific(const ExtendElement& element, TypeAttributes options);
so::Value renderValueSpecific(const RefElement& element, TypeAttributes options);
so::Value renderValue(const IElement& element, TypeAttributes options);

template <typename T>
void renderProperty(so::Object&, const T& element, TypeAttributes)
{
LOG(error) << "invalid property element: " << element.element();
assert(false);
abort();
}

template <typename Element>
Expand All @@ -117,7 +118,7 @@ namespace
so::Value renderValueSpecific(const T& element, TypeAttributes)
{
LOG(error) << "invalid top level element: " << element.element();
assert(false);
abort();
return so::Null{}; // unreachable
}

Expand Down Expand Up @@ -284,6 +285,12 @@ namespace
return renderValue(*merged, options);
}

so::Value renderValueSpecific(const RefElement& element, TypeAttributes options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, could there be other cases where we would render a ref element outside an enum here? Trying to understand if there is perhaps some other test-coverage we should add for other types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should explore that. Also, minim has that generateValue function that probably manifests the same bug.

{
const auto& resolved = resolve(element);
return renderValue(resolved, passFlags(options));
}

} // namespace

namespace
Expand Down
17 changes: 17 additions & 0 deletions test/fixtures/mson/issue-709-b.apib
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# GET /

+ Response 200 (application/json)
+ Attributes
+ code (Post Code)

# Data Structures

## Post Code (enum)

+ Include East Code

## East Code (enum)

+ EC2A
+ E1

Loading