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

Add support for ternary Removable type, to facilitate "merge-patch"-like usecases #1151

Closed
wants to merge 5 commits into from

Conversation

Baccata
Copy link
Contributor

@Baccata Baccata commented Aug 14, 2023

Adds a smithy4s.Removable construct, to help model patchable data where a field can be in three different states :

  1. present
  2. absent
  3. removed

At the schema level, it is modelled as Option[Option[A]] which prevents yet another impactful change. The alloy#nullable trait is also expected to be applied on the field. Without it the deserialisation cannot distinguish between Option[None] and None.

At the codegen level, the signal to render Removable is the presence of the alloy#nullable trait on a field that's not also annotated with smithy.api#required.

TODOs

  • In alloy, add the alloy#nullable to the list of traits known by the simpleRestJson protocol
  • See whether the addition of compliance tests is doable (it may not be the case, considering how smithy works)
  • add documentation in both alloy and smithy4s
  • add support in the dynamic module

@@ -209,12 +210,13 @@ class DocumentDecoderSchemaVisitor(
def option[A](schema: Schema[A]): DocumentDecoder[Option[A]] =
new DocumentDecoder[Option[A]] {
val decoder = schema.compile(self)
val aIsNullable = schema.hints.get(Nullable).isDefined && schema.isOption
Copy link
Contributor

Choose a reason for hiding this comment

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

minor we have schema.hints.has which might make this easier to read

@Baccata Baccata changed the title Add Removable type Add support for ternary Removable type, to facilitate "merge-patch"-like usecases Aug 16, 2023
@lewisjkl
Copy link
Contributor

In alloy, add the alloy#nullable to the list of traits known by the simpleRestJson protocol

If I'm not mistaken, this means we'll need to do a minor version bump (rather than patch) as to not mess with any existing functionality. I think it probably wouldn't mess anything up, but I'm not positive since it will change the HintMasks we derive from the protocolTraits

@Baccata
Copy link
Contributor Author

Baccata commented Feb 27, 2024

Superseded by #1408

@Baccata Baccata closed this Feb 27, 2024
@Baccata Baccata deleted the add-removable-type branch February 27, 2024 15:51
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.

4 participants