-
Notifications
You must be signed in to change notification settings - Fork 507
feat: Add ResourceLink #341
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bzsurbhi,
Please check my comments and let me know what do you think?
@@ -1601,6 +1605,18 @@ public Double priority() { | |||
} | |||
} | |||
|
|||
@JsonInclude(JsonInclude.Include.NON_ABSENT) | |||
@JsonIgnoreProperties(ignoreUnknown = true) | |||
public record ResourceLink( // @formatter:off |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, the ResourceLink
content is equivalent to the existing Resource
type.
Unfortunately, Java doesn't allow record inheritance. Perhaps we should consider a common interface (not sure how to call it? Perhaps ResourceType ) that both Resource and ResourceLink would implement.
Also it will be useful to have Builder for the ResourceLink similar to the Resource's builder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, I'll push an update
- Add ResourceContent interface with common resource metadata methods - Implement ResourceContent in existing Resource class - Add new ResourceLink record class implementing Content and ResourceContent - Update Content interface to support ResourceLink with "resource_link" type (breaking!) - Add tests for ResourceLink serialization/deserialization - Update test expectations to include new resource_link type Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Thank you @bzsurbhi |
rebased, squashed, minor adjustments and merged at 2f55dd0 |
Implement
ResourceLink
as defined in the specMotivation and Context
Adds a missing
ResourceLink
type from the most recent specification versionHow Has This Been Tested?
Added unit tests
Breaking Changes
None
Types of changes
Checklist
Additional context