diff --git a/CHANGELOG.md b/CHANGELOG.md index 758688defa..0e2d9b828c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ When updating the changelog, remember to be very clear about what behavior has c and what APIs have changed, if applicable. ## [Unreleased] +- For entity level action request builders, issue a sensible error if the id is null. ## [29.22.10] - 2021-10-20 - SmoothRateLimiter - do not double-count execution delays on setRate diff --git a/r2-netty/src/main/java/com/linkedin/r2/netty/handler/http2/Http2ProtocolUpgradeHandler.java b/r2-netty/src/main/java/com/linkedin/r2/netty/handler/http2/Http2ProtocolUpgradeHandler.java index 2d78497590..8048671c6d 100644 --- a/r2-netty/src/main/java/com/linkedin/r2/netty/handler/http2/Http2ProtocolUpgradeHandler.java +++ b/r2-netty/src/main/java/com/linkedin/r2/netty/handler/http2/Http2ProtocolUpgradeHandler.java @@ -78,7 +78,7 @@ public void channelActive(ChannelHandlerContext ctx) public static void processChannelActive(ChannelHandlerContext ctx, Logger log, ChannelPromise upgradePromise) { - // For an upgrade request, clients should use an OPTIONS request for path “*” or a HEAD request for “/”. + // For an upgrade request, clients should use an OPTIONS request for path "*" or a HEAD request for "/". // RFC: https://tools.ietf.org/html/rfc7540#section-3.2 // Implementation detail: https://http2.github.io/faq/#can-i-implement-http2-without-implementing-http11 final DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.OPTIONS, "*"); diff --git a/restli-client/src/main/java/com/linkedin/restli/client/ActionRequestBuilder.java b/restli-client/src/main/java/com/linkedin/restli/client/ActionRequestBuilder.java index 32770e80ee..301c2873cd 100644 --- a/restli-client/src/main/java/com/linkedin/restli/client/ActionRequestBuilder.java +++ b/restli-client/src/main/java/com/linkedin/restli/client/ActionRequestBuilder.java @@ -241,6 +241,11 @@ public ActionRequest build() } + protected boolean hasId() + { + return _id != null; + } + private Map, Object> buildReadOnlyActionParameters() { return buildReadOnlyActionParameters(_actionParams); diff --git a/restli-client/src/main/java/com/linkedin/restli/client/base/EntityActionRequestBuilderBase.java b/restli-client/src/main/java/com/linkedin/restli/client/base/EntityActionRequestBuilderBase.java new file mode 100644 index 0000000000..fd1c50ac7e --- /dev/null +++ b/restli-client/src/main/java/com/linkedin/restli/client/base/EntityActionRequestBuilderBase.java @@ -0,0 +1,48 @@ +/* + Copyright (c) 2021 LinkedIn Corp. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package com.linkedin.restli.client.base; + +import com.linkedin.restli.client.ActionRequest; +import com.linkedin.restli.client.RestliRequestOptions; +import com.linkedin.restli.common.ResourceSpec; + +/** + * The abstract base class for all generated request builders classes for entity-level actions. + * + * For entity-level actions the action is being performed against a specific record which means + * that the key/id must be present in the request to identify which record to perform the action + * on. This class adds validation when the request is built that the id was set to a non-null + * value. + */ +public abstract class EntityActionRequestBuilderBase> + extends ActionRequestBuilderBase +{ + protected EntityActionRequestBuilderBase(String baseUriTemplate, Class elementClass, ResourceSpec resourceSpec, + RestliRequestOptions requestOptions) + { + super(baseUriTemplate, elementClass, resourceSpec, requestOptions); + } + + @Override + public ActionRequest build() { + if (!hasId()) + { + throw new IllegalStateException("Entity-level action request is missing required id value."); + } + return super.build(); + } +} diff --git a/restli-int-test-api/src/main/idl/com.linkedin.restli.examples.greetings.client.greeting.restspec.json b/restli-int-test-api/src/main/idl/com.linkedin.restli.examples.greetings.client.greeting.restspec.json index 93aa51d1e4..fad3ec96b6 100644 --- a/restli-int-test-api/src/main/idl/com.linkedin.restli.examples.greetings.client.greeting.restspec.json +++ b/restli-int-test-api/src/main/idl/com.linkedin.restli.examples.greetings.client.greeting.restspec.json @@ -27,6 +27,22 @@ "type" : "int" } ], "returns" : "int" + }, { + "name" : "exampleActionThatIsExplicitlyAnyLevel", + "doc" : "An example action on the greeting which is explicitly set to any-level.", + "parameters" : [ { + "name" : "param1", + "type" : "int" + } ], + "returns" : "int" + }, { + "name" : "exampleActionThatIsExplicitlyEntityLevel", + "doc" : "An example action on the greeting which is explicitly set to entity-level.", + "parameters" : [ { + "name" : "param1", + "type" : "int" + } ], + "returns" : "int" }, { "name" : "exceptionTest", "doc" : "An example action throwing an exception." diff --git a/restli-int-test-api/src/main/snapshot/com.linkedin.restli.examples.greetings.client.greeting.snapshot.json b/restli-int-test-api/src/main/snapshot/com.linkedin.restli.examples.greetings.client.greeting.snapshot.json index 6b8c13e1e9..078883a39c 100644 --- a/restli-int-test-api/src/main/snapshot/com.linkedin.restli.examples.greetings.client.greeting.snapshot.json +++ b/restli-int-test-api/src/main/snapshot/com.linkedin.restli.examples.greetings.client.greeting.snapshot.json @@ -57,6 +57,22 @@ "type" : "int" } ], "returns" : "int" + }, { + "name" : "exampleActionThatIsExplicitlyAnyLevel", + "doc" : "An example action on the greeting which is explicitly set to any-level.", + "parameters" : [ { + "name" : "param1", + "type" : "int" + } ], + "returns" : "int" + }, { + "name" : "exampleActionThatIsExplicitlyEntityLevel", + "doc" : "An example action on the greeting which is explicitly set to entity-level.", + "parameters" : [ { + "name" : "param1", + "type" : "int" + } ], + "returns" : "int" }, { "name" : "exceptionTest", "doc" : "An example action throwing an exception." diff --git a/restli-int-test-server/src/main/java/com/linkedin/restli/examples/greetings/server/RootSimpleResource.java b/restli-int-test-server/src/main/java/com/linkedin/restli/examples/greetings/server/RootSimpleResource.java index 336d23b5b5..1c656cb39f 100644 --- a/restli-int-test-server/src/main/java/com/linkedin/restli/examples/greetings/server/RootSimpleResource.java +++ b/restli-int-test-server/src/main/java/com/linkedin/restli/examples/greetings/server/RootSimpleResource.java @@ -22,6 +22,7 @@ import com.linkedin.restli.common.PatchRequest; import com.linkedin.restli.examples.greetings.api.Greeting; import com.linkedin.restli.examples.greetings.api.Tone; +import com.linkedin.restli.server.ResourceLevel; import com.linkedin.restli.server.RestLiServiceException; import com.linkedin.restli.server.UpdateResponse; import com.linkedin.restli.server.annotations.Action; @@ -96,6 +97,24 @@ public int exampleAction(@ActionParam("param1") int param1) return param1 * 10; } + /** + * An example action on the greeting which is explicitly set to entity-level. + */ + @Action(name="exampleActionThatIsExplicitlyEntityLevel", resourceLevel=ResourceLevel.ENTITY) + public int exampleActionThatIsExplicitlyEntityLevel(@ActionParam("param1") int param1) + { + return param1 * 11; + } + + /** + * An example action on the greeting which is explicitly set to any-level. + */ + @Action(name="exampleActionThatIsExplicitlyAnyLevel", resourceLevel=ResourceLevel.ANY) + public int exampleActionThatIsExplicitlyAnyLevel(@ActionParam("param1") int param1) + { + return param1 * 12; + } + /** * An example action throwing an exception. */ diff --git a/restli-int-test/src/test/java/com/linkedin/restli/examples/TestGreetingsClient.java b/restli-int-test/src/test/java/com/linkedin/restli/examples/TestGreetingsClient.java index cd63a25b47..86ca692ceb 100644 --- a/restli-int-test/src/test/java/com/linkedin/restli/examples/TestGreetingsClient.java +++ b/restli-int-test/src/test/java/com/linkedin/restli/examples/TestGreetingsClient.java @@ -121,6 +121,14 @@ public void testActionRequestOptionsPropagation(RootBuilderWrapper builders) + { + builders.action("SomeAction").build(); + } + @Test(dataProvider = com.linkedin.restli.internal.common.TestConstants.RESTLI_PROTOCOL_1_2_PREFIX + "requestBuilderDataProvider") public void testGetRequestOptionsPropagation(RootBuilderWrapper builders) { diff --git a/restli-int-test/src/test/java/com/linkedin/restli/examples/TestSimpleResourceHierarchy.java b/restli-int-test/src/test/java/com/linkedin/restli/examples/TestSimpleResourceHierarchy.java index d51049fa95..d90464e418 100644 --- a/restli-int-test/src/test/java/com/linkedin/restli/examples/TestSimpleResourceHierarchy.java +++ b/restli-int-test/src/test/java/com/linkedin/restli/examples/TestSimpleResourceHierarchy.java @@ -160,6 +160,24 @@ public void testRootSimpleResourceIntAction(RootBuilderWrapper b Assert.assertEquals(responseFuture.getResponse().getEntity().intValue(), 10); } + @Test(dataProvider = com.linkedin.restli.internal.common.TestConstants.RESTLI_PROTOCOL_1_2_PREFIX + "requestBuilderDataProvider") + public void testRootSimpleResourceEntityAction(RootBuilderWrapper builders) throws RemoteInvocationException + { + Request request = builders.action("ExampleActionThatIsExplicitlyEntityLevel").setActionParam("Param1", 3).build(); + ResponseFuture responseFuture = getClient().sendRequest(request); + Assert.assertEquals(responseFuture.getResponse().getStatus(), 200); + Assert.assertEquals(responseFuture.getResponse().getEntity().intValue(), 33); + } + + @Test(dataProvider = com.linkedin.restli.internal.common.TestConstants.RESTLI_PROTOCOL_1_2_PREFIX + "requestBuilderDataProvider") + public void testRootSimpleResourceAnyAction(RootBuilderWrapper builders) throws RemoteInvocationException + { + Request request = builders.action("ExampleActionThatIsExplicitlyAnyLevel").setActionParam("Param1", 3).build(); + ResponseFuture responseFuture = getClient().sendRequest(request); + Assert.assertEquals(responseFuture.getResponse().getStatus(), 200); + Assert.assertEquals(responseFuture.getResponse().getEntity().intValue(), 36); + } + @Test(dataProvider = com.linkedin.restli.internal.common.TestConstants.RESTLI_PROTOCOL_1_2_PREFIX + "requestBuilderDataProvider") public void testRootSimpleResourceActionException(RootBuilderWrapper builders) throws RemoteInvocationException { diff --git a/restli-tools/src/main/java/com/linkedin/restli/tools/clientgen/JavaRequestBuilderGenerator.java b/restli-tools/src/main/java/com/linkedin/restli/tools/clientgen/JavaRequestBuilderGenerator.java index cd557fd8a1..791c4478ab 100644 --- a/restli-tools/src/main/java/com/linkedin/restli/tools/clientgen/JavaRequestBuilderGenerator.java +++ b/restli-tools/src/main/java/com/linkedin/restli/tools/clientgen/JavaRequestBuilderGenerator.java @@ -42,6 +42,7 @@ import com.linkedin.pegasus.generator.JavaDataTemplateGenerator; import com.linkedin.pegasus.generator.TemplateSpecGenerator; import com.linkedin.pegasus.generator.spec.ClassTemplateSpec; +import com.linkedin.restli.client.ActionRequestBuilder; import com.linkedin.restli.client.OptionsRequestBuilder; import com.linkedin.restli.client.RestliRequestOptions; import com.linkedin.restli.client.base.ActionRequestBuilderBase; @@ -57,6 +58,7 @@ import com.linkedin.restli.client.base.CreateIdEntityRequestBuilderBase; import com.linkedin.restli.client.base.CreateIdRequestBuilderBase; import com.linkedin.restli.client.base.DeleteRequestBuilderBase; +import com.linkedin.restli.client.base.EntityActionRequestBuilderBase; import com.linkedin.restli.client.base.FindRequestBuilderBase; import com.linkedin.restli.client.base.GetAllRequestBuilderBase; import com.linkedin.restli.client.base.GetRequestBuilderBase; @@ -1378,7 +1380,9 @@ private void generateActions(JDefinedClass facadeClass, { for (ActionSchema action : resourceActions) { - generateActionMethod(facadeClass, baseUriExpr, _voidClass, action, resourceSpecField, resourceName, pathKeys, pathKeyTypes, assocKeyTypes, pathToAssocKeys, requestOptionsExpr, rootPath); + generateActionMethod(facadeClass, baseUriExpr, _voidClass, ActionRequestBuilderBase.class, action, + resourceSpecField, resourceName, pathKeys, pathKeyTypes, assocKeyTypes, pathToAssocKeys, requestOptionsExpr, + rootPath); } } @@ -1386,7 +1390,9 @@ private void generateActions(JDefinedClass facadeClass, { for (ActionSchema action : entityActions) { - generateActionMethod(facadeClass, baseUriExpr, keyClass, action, resourceSpecField, resourceName, pathKeys, pathKeyTypes, assocKeyTypes, pathToAssocKeys, requestOptionsExpr, rootPath); + generateActionMethod(facadeClass, baseUriExpr, keyClass, EntityActionRequestBuilderBase.class, action, + resourceSpecField, resourceName, pathKeys, pathKeyTypes, assocKeyTypes, pathToAssocKeys, requestOptionsExpr, + rootPath); } } } @@ -1394,6 +1400,7 @@ private void generateActions(JDefinedClass facadeClass, private void generateActionMethod(JDefinedClass facadeClass, JExpression baseUriExpr, JClass keyClass, + Class builderParentClass, ActionSchema action, JVar resourceSpecField, String resourceName, @@ -1406,7 +1413,7 @@ private void generateActionMethod(JDefinedClass facadeClass, throws JClassAlreadyExistsException { final JClass returnType = getActionReturnType(facadeClass, action.getReturns()); - final JClass vanillaActionBuilderClass = getCodeModel().ref(ActionRequestBuilderBase.class).narrow(keyClass, returnType); + final JClass vanillaActionBuilderClass = getCodeModel().ref(builderParentClass).narrow(keyClass, returnType); final String actionName = action.getName(); final String actionBuilderClassName = CodeUtil.capitalize(resourceName) + "Do" + CodeUtil.capitalize(actionName) + METHOD_BUILDER_SUFFIX.get(_version);