Skip to content

Add support for 1.0 downgrades and serialization#1403

Merged
mtdowling merged 1 commit intomainfrom
add-1.0-serialization-option
Sep 14, 2022
Merged

Add support for 1.0 downgrades and serialization#1403
mtdowling merged 1 commit intomainfrom
add-1.0-serialization-option

Conversation

@mtdowling
Copy link
Copy Markdown
Member

This commit adds the ability to downgrade the in-memory semantic model to 1.0 models, and adds support for serializing 1.0 JSON AST models to ModelSerializer.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This commit adds the ability to downgrade the in-memory semantic
model to 1.0 models, and adds support for serializing 1.0 JSON
AST models to ModelSerializer.
@mtdowling mtdowling requested a review from a team as a code owner September 14, 2022 20:40
return this;
}

@Override
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The fluent interface here needed this override for it to work

@mtdowling mtdowling merged commit 6123a95 into main Sep 14, 2022

Node.assertEquals(actual, expected);

// Now validate the file is serialized correctly when downgraded to 1.0.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would we be able to have a test that starts a 1.0 model, loads it and serializes it as 1.0 and compares?

Comment on lines +122 to +123
for (StructureShape structure : model.getStructureShapes()) {
for (MemberShape member : structure.getAllMembers().values()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could have been getMemberShapesWithTrait like above?

* @param model Model to downgrade.
* @return Returns the downgraded model.
*/
public Model downgradeToV1(Model model) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The javadoc does say 'remove' a bunch, but probably worth an explicit callout that this is lossy.

+ "structure Struct {\n"
+ " @default(10)\n"
+ " foo: MyInteger\n"
+ " baz: PrimitiveInteger = null\n"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good to add a member which has the zero value and make assertions on that

@mtdowling mtdowling deleted the add-1.0-serialization-option branch October 24, 2022 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants