Require required attributes#5
Open
ehannes wants to merge 3 commits into
Open
Conversation
The DSL auto-applied :synthetic when a parse block (or mapper #parse) declared 2+ parameters, but did not do the same for the symmetric multi-param serialise case. Both shapes produce an attribute whose value does not live at @model_attributes[model_name]; only the merge side was being marked. Until now the flag had no readers in lib, so the asymmetry was invisible. It becomes visible the moment any code path branches on synthetic? — applying the flag now keeps that future code correct.
Previously the flag raised MissingAttributeError only when an API response omitted the field; it was silently ignored on save, letting incomplete payloads reach the backend. Serialise now raises before any HTTP request when a required attribute is nil, with the attribute name on the exception.
For attributes with multi-param parse or serialise, :required was a
silent no-op: the parse-side check sat inside the non-source_fields
branch, and the serialise-side check explicitly bypassed any attr
flagged :synthetic. The flag's intent ("don't accept partial data")
was unenforceable for the very shapes most likely to depend on it.
Semantic: marking a synthetic attribute :required means all of its
underlying fields must be present.
* Merge (multi-param parse): all source API fields must be in the
response on parse; if round-trippable, the merged model value
must additionally be non-nil at serialise time.
* Combine (multi-param serialise): all named model attributes must
be non-nil at serialise time. Parse-side check does not apply.
* Split (single-param bare block extracting from one API field):
the underlying API field must be present on parse.
:read_only continues to skip the serialise-time check, matching the
single-field semantics.
Documents the previously-undocumented combine pattern (multi-param
serialise) in the README, alongside merge and split.
There was a problem hiding this comment.
Pull request overview
This PR tightens :required attribute semantics in RestEasy::Resource by enforcing requiredness not only when parsing inbound API payloads, but also when serialising outbound payloads (including synthetic/derived attributes), and documents the updated behavior.
Changes:
- Enforce
:requiredonserialise(and thereforesave/to_api), raisingMissingAttributeErrorwith#attribute_name. - Enforce
:requiredfor synthetic attributes on parse/serialise, and auto-tag multi-parameterserialisedefinitions as:synthetic. - Expand specs, README documentation, and changelog entries to cover the new rules and synthetic patterns (merge/combine/split).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| spec/rest_easy/resource_spec.rb | Adds comprehensive coverage for :required across parse/stub/serialise/save and synthetic patterns, plus synthetic auto-detection. |
| lib/rest_easy/resource.rb | Implements serialise-time required checks and required enforcement for synthetic parse paths; auto-applies :synthetic for multi-parameter serialise. |
| README.md | Updates :required documentation and adds detailed explanations/examples for merge/combine/split behavior. |
| CHANGELOG.md | Records the behavioral change for :required and synthetic auto-detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+699
to
+701
| if attr_def.required? && raw_values.any?(&:nil?) | ||
| raise MissingAttributeError.new(model_name) | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.