Skip to content

Commit

Permalink
Throw an exception when an entity-level action would be invoked witho…
Browse files Browse the repository at this point in the history
…ut an id.

Entity-level actions are performed on a specific resource, and so the id value
in the request is mandatory. Today, if the client forgets to call .id(...) to
supply a value the request can be built and sent to the server where it will
cause an error with a mysterious error message that implies that there is no
action of that name, i.e.
"POST operation named myAction not supported on resource '...' URI: '...'"

Here we fix that so that
a) The problem is caught on the client side during the RequestBuilder.build()
   method call so that the request cannot be sent to the server.
b) A sensible error message is given where the stack trace's line number will
   lead the developer straight to the root of the problem:
   "Missing or null id value; we cannot invoke this entity-level action."
  • Loading branch information
haroldl committed Nov 11, 2021
1 parent e779b9f commit 2328f99
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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, "*");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ public ActionRequest<V> build()

}

protected boolean isIdPresent()
{
return _id != null;
}

private Map<FieldDef<?>, Object> buildReadOnlyActionParameters()
{
return buildReadOnlyActionParameters(_actionParams);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
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.
*/
public abstract class EntityActionRequestBuilderBase<K, V, RB extends ActionRequestBuilderBase<K, V, RB>>
extends ActionRequestBuilderBase<K, V, RB>
{
protected EntityActionRequestBuilderBase(String baseUriTemplate, Class<V> elementClass, ResourceSpec resourceSpec,
RestliRequestOptions requestOptions)
{
super(baseUriTemplate, elementClass, resourceSpec, requestOptions);
}

@Override
public ActionRequest<V> build() {
if (!isIdPresent())
{
throw new IllegalStateException("Missing or null id value; we cannot invoke this entity-level action.");
}
return super.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ public void testActionRequestOptionsPropagation(RootBuilderWrapper<Long, Greetin
Assert.assertEquals(request.getRequestOptions(), builders.getRequestOptions());
}

@Test(dataProvider = com.linkedin.restli.internal.common.TestConstants.RESTLI_PROTOCOL_1_2_PREFIX + "requestBuilderDataProvider",
expectedExceptions = { IllegalStateException.class },
expectedExceptionsMessageRegExp = "Missing or null id value; we cannot invoke this entity-level action.")
public void testEntityActionRequiresIdInBuild(RootBuilderWrapper<Long, Greeting> builders)
{
builders.action("SomeAction").build();
}

@Test(dataProvider = com.linkedin.restli.internal.common.TestConstants.RESTLI_PROTOCOL_1_2_PREFIX + "requestBuilderDataProvider")
public void testGetRequestOptionsPropagation(RootBuilderWrapper<Long, Greeting> builders)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -1378,22 +1380,27 @@ 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);
}
}

if (entityActions != null)
{
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);
}
}
}

private void generateActionMethod(JDefinedClass facadeClass,
JExpression baseUriExpr,
JClass keyClass,
Class<?> builderParentClass,
ActionSchema action,
JVar resourceSpecField,
String resourceName,
Expand All @@ -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);
Expand Down

0 comments on commit 2328f99

Please sign in to comment.