Support Device Schema Extensions (RFC 9944) - #314
Conversation
Added model classes, extensions, and new fields as defined by RFC 9944. This RFC describes an update to the SCIM standard for managing user IoT devices, and the bulk of these changes have been placed in a new, dedicated "devices" package. The DeviceResource class-level Javadoc contains an introduction to device resources and how to leverage them with this SDK. This change includes support in scim2-sdk-server for evaluating device resource and related object compliance with the SchemaChecker. Note that explicit client changes were not required, as the DeviceResource class can leverage the existing client APIs like any ScimResource object. The release version has been set to 6.1.0-SNAPSHOT. Reviewer: dougbulkley Reviewer: vyhhuang JiraIssue: DS-51992
| @Nullable | ||
| @Attribute(description = | ||
| "The URI of the enterprise device-control endpoint application.", | ||
| isRequired = false, |
There was a problem hiding this comment.
In the RFC this is required. I see your comment explaining the deviation. You might consider noting this deviation in the CHANGELOG.
There was a problem hiding this comment.
I've added an entry to the CHANGELOG for this.
| @Nullable | ||
| @Attribute(description = | ||
| "The subject name for the endpoint application certificate.", | ||
| isRequired = false) |
There was a problem hiding this comment.
The subjectName for CertificateInfo is required and case exact in the RFC tables. But I also found that the RFC is in disagreement with itself because it also states subjectName "...is not required and not case sensitive." An RFC bug?
There was a problem hiding this comment.
I set this as not required because of that line in the RFC. Now that you mention this, though, all other references in the document mark it as required, so it seems very likely that the "is not required" statement is a mistake. I did find it strange that it was technically valid to have a JSON object with a present but empty certificateInfo field.
Since there's strong evidence the intent was for this field to be required, I've updated the implementation to reflect that. I also got rid of the helper method that tracked the awkward state of "null or empty certificateInfo object". Thanks for bringing this up.
vyhhuang
left a comment
vyhhuang
left a comment
There was a problem hiding this comment.
I had a couple of optional suggestions, but these changes look good to me.
| isCaseExact = true, | ||
| referenceTypes = { "EndpointApp" }, | ||
| mutability = AttributeDefinition.Mutability.READ_ONLY) | ||
| @JsonProperty("$ref") |
There was a problem hiding this comment.
Like with the 'deviceControlEnterpriseEndpoint' field in EndpointAppDeviceExtension, it might be worth explicitly stating that this field is not required in the changelog.
| isCaseExact = false, | ||
| mutability = AttributeDefinition.Mutability.READ_WRITE, | ||
| returned = AttributeDefinition.Returned.DEFAULT, | ||
| uniqueness = AttributeDefinition.Uniqueness.NONE, |
There was a problem hiding this comment.
Nit- both of these fields should have the default of AttributeDefinition.Uniqueness.NONE -- is there a reason only one is explicitly marked as such?
There was a problem hiding this comment.
No, I just missed this while cleaning up these references. Thanks for mentioning this.
| // The 'name' field is a human-friendly name for the object. | ||
| String schemaId = | ||
| SchemaUtils.getSchemaIdFromAnnotation(cls); | ||
| SchemaUtils.getSchemaIdFromAnnotation(cls); |
There was a problem hiding this comment.
Uber-nit: unnecessary whitespace change.
There was a problem hiding this comment.
This was a result of a bad revert. I've fixed this.
3 participants
Added model classes, extensions, and new fields as defined by RFC 9944.
This RFC describes an update to the SCIM standard for managing user IoT
devices, and the bulk of these changes have been placed in a new,
dedicated "devices" package. The DeviceResource class-level Javadoc
contains an introduction to device resources and how to leverage them
with this SDK.
This change includes support in scim2-sdk-server for evaluating device
resource and related object compliance with the SchemaChecker. Note that
explicit client changes were not required, as the DeviceResource class
can leverage the existing client APIs like any ScimResource object.
The release version has been set to 6.1.0-SNAPSHOT.
Reviewer: dougbulkley
Reviewer: vyhhuang
JiraIssue: DS-51992