Allow lists being marked as httpPayload

[timocov]

Issue #, if available:

Description of changes:

When developing a custom protocol based on JSON it is not possible to have a list marked as @httpPayload, e.g. for the model:

operation Op {
  output := {
    @httpPayload
    content: StringList
  }
}

list StringList {
  member: String
}

Whilst it is possible to define such model in a custom protocol (with aws.protocols#restJson1 it still validates the model and fails with an error that AWS Protocols don't support it).

I understand why built-in/AWS protocols might not want to support it (e.g. enforcing best practices), but it feels like it should be possible to define this in a custom protocol (especially for backward compatibility when onboarding smithy to an existing services).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[mtdowling]

Have you been able to actually define a model with an httpPayload trait that targets a list? Is this change bigger than this PR and would also require model changes?

[timocov]

Have you been able to actually define a model with an httpPayload trait that targets a list?

@mtdowling yep, and it works just fine. I can share more if you want (but tell me what you're interested in as it might be a bit too much 😂). And this was the only change I made in smithy-java to make it work. This is the protocol and the model I used for my client and it worked just fine:

Details

SimpleRestJsonClientProtocol.java:

public final class SimpleRestJsonClientProtocol extends HttpBindingClientProtocol<AwsEventFrame> {

    private final JsonCodec codec;
    private final HttpErrorDeserializer errorDeserializer;

    /**
     * Constructs a new SimpleRestJsonClientProtocol.
     *
     * @param service The service being called. This is used when finding the discriminator
     *                of documents that use relative shape IDs.
     */
    public SimpleRestJsonClientProtocol(ShapeId service) {
        super(SimpleRestJsonTrait.ID);

        this.codec = JsonCodec.builder()
                .useJsonName(true)
                .useTimestampFormat(true)
                .defaultNamespace(service.getNamespace())
                .build();

        this.errorDeserializer = HttpErrorDeserializer.builder()
                .codec(codec)
                .serviceId(service)
                .build();
    }

    @Override
    public <I extends SerializableStruct, O extends SerializableStruct> HttpRequest createRequest(
            ApiOperation<I, O> operation,
            I input,
            Context context,
            URI endpoint
    ) {
        RequestSerializer serializer = httpBinding().requestSerializer()
                .operation(operation)
                .payloadCodec(payloadCodec())
                .payloadMediaType(payloadMediaType())
                .shapeValue(input)
                .endpoint(endpoint)
                .omitEmptyPayload(omitEmptyPayload())
                .allowEmptyStructPayload(hasStructPayload(input));

        if (operation instanceof InputEventStreamingApiOperation<?, ?, ?> i) {
            serializer.eventEncoderFactory(getEventEncoderFactory(i));
        }

        return serializer.serializeRequest();
    }

    @Override
    public Codec payloadCodec() {
        return codec;
    }

    @Override
    protected String payloadMediaType() {
        return "application/json";
    }

    @Override
    protected HttpErrorDeserializer getErrorDeserializer(Context context) {
        return errorDeserializer;
    }

    @Override
    protected boolean omitEmptyPayload() {
        return true;
    }

    @Override
    protected EventEncoderFactory<AwsEventFrame> getEventEncoderFactory(
            InputEventStreamingApiOperation<?, ?, ?> inputOperation
    ) {
        // Event streaming is not currently supported for simpleRestJson protocol
        throw new UnsupportedOperationException("Event streaming is not yet supported for simpleRestJson protocol");
    }

    /**
     * Checks if the input has a struct payload member.
     */
    private <I extends SerializableStruct> boolean hasStructPayload(I input) {
        var members = input.schema().members();
        for (var member : members) {
            if (member.type().equals(ShapeType.STRUCTURE)
                    && member.hasTrait(TraitKey.HTTP_PAYLOAD_TRAIT)) {
                return true;
            }
        }
        return false;
    }

    /**
     * Factory for creating SimpleRestJsonClientProtocol instances.
     */
    public static final class Factory implements ClientProtocolFactory<SimpleRestJsonTrait> {
        @Override
        public ShapeId id() {
            return SimpleRestJsonTrait.ID;
        }

        @Override
        public ClientProtocol<?, ?> createProtocol(ProtocolSettings settings, SimpleRestJsonTrait trait) {
            return new SimpleRestJsonClientProtocol(settings.service());
        }
    }
}

main.smithy:

@cors
@simpleRestJson
service ServiceName {
    operations: [
        GetObjects
    ]
}

@http(uri: "/v1/path", method: "GET")
operation GetObjects {
    output := {
        @httpPayload
        items: ObjectList
    }
}

list ObjectList {
    member: Object
}

structure Object {
    id: Integer
}

[timocov]

Is this change bigger than this PR and would also require model changes?

In theory it would be possible to use with aws restJson1 protocol, but it has a model validation for this so I didn't investigate it further to keep it consistent with current behavior.

[mtdowling]

Cool. Seems fine then. Can you push up a rebase, and then I'll merge?

[timocov]

@mtdowling done

[mtdowling]

Thanks!