Overriding `Client.call` method

[timocov]

(Originally asked in #1162 (comment))

I'd want to ask the team's vision on Client.call method. Currently it is marked as protected and I understand that the main purpose of this is to allow derived "impl" classes (generated by the codegen) to call it, but also in theory it is possible to override it during the codegen to have some extra logic in it (and still call the original method from the base-class). For example, lets say you're integrating an XRay-like library into clients so that every client call creates a sub-segment, then the implementation of the overridden method can look something like this:

    <I extends SerializableStruct, O extends SerializableStruct> O call(
        I input,
        ApiOperation<I, O> operation,
        RequestOverrideConfig overrideConfig
    ) {
        // note that "ServiceName.OperationName" can be extracted from operation/service, but I omitted it from this example
        return AWSXRay.getGlobalRecorder().createSegment("ServiceName.OperationName", () -> {
            return super.call(input, operation, overrideConfig);
        });
    }

In this case, we're using library's provided "wrapping" function to make a call so that it can handle "start" and "end" correctly. In case of XRay, it provides beginSegment/endSegment methods so it is not quite fair comparison, but not all libraries have that and if it is a 3rd-party library it might not be even possible.

In my use-case, I have a library that manages current "context" state that then later is being used (or can be used) when making actual http requests at the transport level by adding some headers to request. And unfortunately this library has a couple of features so that even if I introduce begin-end-like methods, it won't be possible as it does some extra logic during this "call"-phase that uses library's internals.

So I have the following questions:

• Do you expect that consumers can/should be able to override Client.call method in their derived classes?
• If the answer is yes, would you accept a contribution of:
  • opening some of the RequestOverrideConfig fields to external consumers (or at least the context getter)? Currently every getter in this class is package-private so it is not possible to access them from the outside (e.g. to get the context which was previously set by custom settings set by defaultSettings (similar to

    smithy-java/examples/dynamodb-client/smithy-build.json

    Lines 9 to 11 in b9d6559

    	"defaultSettings": [
    	"software.amazon.smithy.java.aws.client.auth.scheme.sigv4.SigV4Settings"
    	],

    ). I understand that the main approach for this type of tasks should be using an interceptor, but when integrating into an existing system it is not always possible and some libraries require to wrap a call so that the context is correctly "scoped".
  • adding something (not yet sure what exactly, a codesection of some sort?) to the codegen so that it would be possible to do without parsing generated client class and finding a place to insert an override method in there? It is not critical thing I'd say as consumers still would be able to "hack it" from their side, but it would be a nice addition to reduce hacks in consumers code.

Thanks!

[mtdowling]

You can of course override the method, though I want to do something like this:

interceptors can't really do decoration, especially if decoration needs to potentially be re-entrant to the client (multiple calls).

We could do something like

@FunctionalInterface
public interface ClientCallDecorator<C> {
    <I extends SerializableStruct, O extends SerializableStruct> O apply(
        C client,
        ApiOperation<I, O> operation,
        I input,
        RequestOverrideConfig overrideConfig,
        ClientCallInvoker next
    );
}

add to a client builder:

// In ClientConfig.Builder
private ClientCallDecorator<?> decorator;

@SuppressWarnings("unchecked")
public <C> Builder decorator(ClientCallDecorator<C> decorator) {
    this.decorator = decorator;
    return this;
}

then when making a call:

protected <I extends SerializableStruct, O extends SerializableStruct> O call(
    I input, ApiOperation<I, O> operation, RequestOverrideConfig overrideConfig
) {
    if (decorator != null) {
        return decorator.apply(this, operation, input, overrideConfig, this::doCall);
    }
    return doCall(input, operation, overrideConfig);
}

An example decorator (hypothetical)

public class CachingDecorator implements ClientCallDecorator<DynamoDBClient> {
    private final Map<String, SerializableStruct> cache = new ConcurrentHashMap<>();

    @Override
    public SerializableStruct apply(
        DynamoDBClient client,
        ApiOperation<?, ?> operation,
        SerializableStruct input,
        RequestOverrideConfig overrideConfig,
        ClientCallInvoker next
    ) {
        // Only cache GetItem calls
        if (input instanceof GetItemInput getItem) {
            var key = getItem.tableName() + ":" + getItem.key();
            var cached = cache.get(key);
            if (cached != null) {
                return cached;
            }
            var result = next.invoke(input, operation, overrideConfig);
            cache.put(key, result);
            return result;
        }

        // Invalidate on writes
        if (input instanceof PutItemInput putItem) {
            cache.remove(putItem.tableName() + ":" + putItem.key());
        }

        return next.invoke(input, operation, overrideConfig);
    }
}