Skip to content

Conversation

@dplaha-coursera
Copy link
Contributor

@dplaha-coursera dplaha-coursera commented Mar 28, 2022

Duplicate of #283 with two changes:

  • Updates copyright notices on copied source files.
  • Applies enum relaxed validation to StringKey deserialization as well. That is, StringKeys containing unknown enums will also deserialize them to $UNKNOWN instead of throwing an exception.

Original PR Description

This PR changes the validation logic used by courier to ignore unrecognized enum values instead of returning a JsError.

I copied the 3.1.1 version of ValidateDataAgainstSchema into the naptime repo, re-writing from java to scala. This class has a lot of options, so I removed support for options we're not using.

To begin with, I just changed the invocation from the rest.li version to this version of the class in CourierSerializer. This was the only change required to get the framework to deserialize unknown enums in arrays or records without throwing an exception. There are a few other invocations of ValidateDataAgainstSchema.validate scattered across the naptime repo, but I wanted to limit the blast radius of this PR.

@dplaha-coursera dplaha-coursera changed the title Deep/courier deserialize unknown enum loosen validation to support unrecognized enum values Mar 28, 2022
@dplaha-coursera dplaha-coursera marked this pull request as ready for review March 29, 2022 18:08
@dplaha-coursera dplaha-coursera force-pushed the deep/courier_deserialize_unknown_enum branch from 65b3341 to 2cca58b Compare April 4, 2022 20:53
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dplaha-coursera
Copy link
Contributor Author

dplaha-coursera commented Apr 21, 2022

Tested 0.11.6-alpha5 in infra-services -- verified that deserializing unknown enums no longer throws an exception.

@dguo-coursera dguo-coursera merged commit 886626e into coursera:master Apr 21, 2022
dplaha-coursera added a commit to dplaha-coursera/naptime that referenced this pull request Apr 25, 2022
dguo-coursera pushed a commit that referenced this pull request May 19, 2022
…#288)

* Revert "loosen validation to support unrecognized enum values (#285)"

This reverts commit 886626e.

# Conflicts:
#	version.sbt

* version bump
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