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

RUMM-2266 Enable Session Replay models generation #938

Merged

Conversation

ncreated
Copy link
Member

What and why?

📦 This PR fixes 2 problems discovered when running models generation for Session Replay schemas. With this fix, the generator makes successful run on session-replay-mobile-events-format.json and prints Swift code.

This PR doesn't yet:

  • Decorate SR models code appropriately. Similar to RUM, there are several things we may want to adjust in produced code (e.g. naming, or reusing common types) to make it more convenient to work with.
  • Restructure rum-models-generator package to include separate generation tracks for RUM and SR.

Both elements will be delivered in next PRs.

How?

First, I enhanced JSON schema reader with inferring type if it is not explicitly defined. This is the case of mutation-data-schema.json which defines following array items without declaring type: object:

"items": {
   "required": [
       "wireframe"
   ],
   "properties": {
       "previousId": {
           "type": "integer",
           "description": "The previous wireframe id next or after which this new wireframe is drawn or attached to, respectively.",
           "readOnly": true
       },
       "wireframe": {
           "$ref": "wireframe-schema.json",
           "description": "The new wireframe that needs to be added to the screen.",
           "readOnly": true
       }
   }
}

In such case ☝️, we can infer that item is of type: object, because it defines properties.

Alternatively, we could consider updating mutation-data-schema.json with type information. I don't do it for two reasons:

  • couldn't find anywhere in JSON schema doc if type is a mandatory field (seems not);
  • other generators (Kotlin + TS) seem to handle this case, so it sounds reasonable to include it in ours.

Second, there was one generation branch in oneOf resolution marked as "not yet supported". In this PR I'm adding support to mixing JSONObject and JSONOneOf definitions in a single oneOf.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@ncreated ncreated requested a review from a team as a code owner July 22, 2022 12:34
@ncreated ncreated self-assigned this Jul 22, 2022
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

👌

@ncreated ncreated merged commit edee34f into develop Jul 26, 2022
@ncreated ncreated deleted the ncreated/RUMM-2266-fix-problems-in-sr-models-generation branch July 26, 2022 09:44
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.

2 participants