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

Allow both @extension and @grpcExtension extensions in schema validation #1009

Merged
merged 3 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ and what APIs have changed, if applicable.

## [Unreleased]

## [29.58.0] - 2024-07-11
- Allow both @extension and @grpcExtension extensions in schema validation

## [29.57.2] - 2024-06-17
- Update grpc version to 1.59.1 and protobuf to 3.24.0

Expand Down Expand Up @@ -5707,7 +5710,8 @@ patch operations can re-use these classes for generating patch messages.

## [0.14.1]

[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.57.2...master
[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.58.0...master
[29.58.0]: https://github.com/linkedin/rest.li/compare/v29.57.2...v29.58.0
[29.57.2]: https://github.com/linkedin/rest.li/compare/v29.57.1...v29.57.2
[29.57.1]: https://github.com/linkedin/rest.li/compare/v29.57.0...v29.57.1
[29.57.0]: https://github.com/linkedin/rest.li/compare/v29.56.1...v29.57.0
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version=29.57.2
version=29.58.0
group=com.linkedin.pegasus
org.gradle.configureondemand=true
org.gradle.parallel=true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@
* 1. The extension schema is a valid schema.
* 2. The extension schema name has to follow the naming convention: <baseSchemaName> + "Extensions"
* 3. The extension schema can only include the base schema.
* 4. The extension schema's field annotation keys must be in the "extension" or "grpcExtension" namespaces (but not both).
* 5. The extension schema's field annotations must conform to {@link ExtensionSchemaAnnotation} or {@link GrpcExtensionAnnotation}.
* 4. The extension schema's field annotation keys must be in the "extension" and/or "grpcExtension" namespaces
* 5. The extension schema's field annotations must conform to {@link ExtensionSchemaAnnotation} and/or {@link GrpcExtensionAnnotation}.
* 6. The extension schema's fields can only be Typeref or array of Typeref.
* 7. The extension schema's field schema's annotation keys must be in the "resourceKey" and/or "grpcService" namespaces.
* 8. The extension schema's field annotation versionSuffix value has to match the versionSuffix value in "resourceKey"/"grpcService" annotation on the field schema.
Expand Down Expand Up @@ -232,7 +232,7 @@ private static void checkExtensionSchemaFields(List<RecordDataSchema.Field> exte
{
validateRestLiExtensionField(field);
}
else if (properties.containsKey(GRPC_EXTENSION_ANNOTATION_NAMESPACE))
if (properties.containsKey(GRPC_EXTENSION_ANNOTATION_NAMESPACE))
{
validateGrpcExtensionField(field);
}
Expand All @@ -242,12 +242,6 @@ else if (properties.containsKey(GRPC_EXTENSION_ANNOTATION_NAMESPACE))
private static void validateRestLiExtensionField(RecordDataSchema.Field field)
throws InvalidExtensionSchemaException {
Map<String, Object> properties = field.getProperties();
// Validate that it doesn't also contain gRPC downstream info
if (properties.containsKey(GRPC_EXTENSION_ANNOTATION_NAMESPACE))
{
throw new InvalidExtensionSchemaException("The extension schema field '"
+ field.getName() + "' cannot be annotated with both 'extension' and 'grpcExtension'");
}

// Validate the actual content/structure of the annotation value
validateFieldAnnotation(properties.get(EXTENSION_ANNOTATION_NAMESPACE), new ExtensionSchemaAnnotation().schema());
Expand All @@ -262,12 +256,6 @@ private static void validateRestLiExtensionField(RecordDataSchema.Field field)
private static void validateGrpcExtensionField(RecordDataSchema.Field field)
throws InvalidExtensionSchemaException {
Map<String, Object> properties = field.getProperties();
// Validate that it doesn't also contain Rest.li downstream info
if (properties.containsKey(EXTENSION_ANNOTATION_NAMESPACE))
{
throw new InvalidExtensionSchemaException("The extension schema field '"
+ field.getName() + "' cannot be annotated with both 'extension' and 'grpcExtension'");
}

// Validate the actual content/structure of the annotation value
validateFieldAnnotation(properties.get(GRPC_EXTENSION_ANNOTATION_NAMESPACE), new GrpcExtensionAnnotation().schema());
Expand Down
10 changes: 10 additions & 0 deletions restli-tools/src/test/extensions/validCase/BazExtensions.pdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Valid extension schema:
* The co-existence of @extension and @grpcExtension is allowed
*/
record BazExtensions includes Baz {
@extension.using = "finder: test"
@grpcExtension.rpc = "get"
Copy link
Contributor

Choose a reason for hiding this comment

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

what would be the expectation for translating this annotation to proto during migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The proto schema would look something like this.

@grpcExtension.versionSuffix = "V2"
testField: array[DummyKeyWithGrpc]
}
42 changes: 42 additions & 0 deletions restli-tools/src/test/pegasus/DummyKeyWithGrpc.pdl
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* A test schema which is used as a field type in extension schema.
*/
@resourceKey = [ {
"keyConfig" : {
"keys" : {
"profilesId" : {
"assocKey" : {
"authorId" : "fabricName",
"objectId" : "sessionId"
}
}
}
},
"entity" : "Profile",
"resourcePath" : "/profiles/{profilesId}"
}, {
"keyConfig" : {
"keys" : {
"profilesId" : {
"assocKey" : {
"authorId" : "fabricName",
"objectId" : "sessionId"
}
}
}
},
"entity" : "ProfileV2",
"resourcePath" : "/profilesV2/{profilesId}",
"versionSuffix" : "V2"
} ]
@grpcService = [ {
"entity" : "proto.com.linkedin.Profile"
"rpc" : "get",
"service" : "proto.com.linkedin.ProfileService"
}, {
"entity" : "proto.com.linkedin.ProfileV2"
"rpc" : "get",
"service" : "proto.com.linkedin.ProfileServiceV2"
"versionSuffix": "V2"
} ]
typeref DummyKeyWithGrpc = string
Loading