Serializing default values causes failure in EC2 DescribeRequest

[bpholt]

Using the generated EC2 client's describeInstance method, set a list of Instance IDs and attempt to make a request. It will fail because the default MaxResults value is set on the request as well, which is forbidden by the AWS API.

Reproduction:

import cats.*
import cats.effect.*
import cats.syntax.all.*
import com.amazonaws.ec2.*
import org.http4s.*
import org.http4s.client.{Client, middleware}
import org.http4s.ember.client.EmberClientBuilder
import smithy4s.aws.*
import smithy4s.aws.kernel.AwsRegion

object TestApp extends ResourceApp.Simple:
  private val id = InstanceId("i-00000000000000000")
  override def run: Resource[IO, Unit] =
    EmberClientBuilder.default[IO].build
      .map(middleware.Logger(logHeaders = true, logBody = true))
      .flatMap(AwsEnvironment.default(_, AwsRegion.US_WEST_2))
      .flatMap(AwsClient(EC2, _))
      .evalMap(_.describeInstances(instanceIds = Option(List(id))))
      .evalMap(IO.println(_))
      .void

Running this results in

HTTP/1.1 POST https://ec2.us-west-2.amazonaws.com/ Headers(Content-Length: 102, Authorization: <REDACTED>, Content-Type: application/x-www-form-urlencoded, host: ec2.us-west-2.amazonaws.com, X-Amz-Date: 20231207T004525Z, X-Amz-Target: AmazonEC2.DescribeInstances) body="Action=DescribeInstances&Version=2016-11-15&DryRun=false&MaxResults=0&InstanceId.1=i-00000000000000000"
HTTP/1.1 400 Bad Request Headers(x-amzn-RequestId: redacted, Cache-Control: no-cache, no-store, Strict-Transport-Security: max-age=31536000; includeSubDomains, vary: accept-encoding, Content-Type: text/xml;charset=UTF-8, Transfer-Encoding: chunked, Date: Thu, 07 Dec 2023 00:45:26 GMT, Connection: close, Server: AmazonEC2) body="<?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>InvalidParameterCombination</Code><Message>The parameter instancesSet cannot be used with the parameter maxResults</Message></Error></Errors><RequestID>redacted</RequestID></Response>"
smithy4s.http.UnknownErrorResponse: status 400, headers: HashMap(Cache-Control -> List(no-cache, no-store), x-amzn-RequestId -> List(redacted), Connection -> List(close), Strict-Transport-Security -> List(max-age=31536000; includeSubDomains), vary -> List(accept-encoding), Date -> List(Thu, 07 Dec 2023 00:45:26 GMT), Content-Type -> List(text/xml;charset=UTF-8), Server -> List(AmazonEC2), Transfer-Encoding -> List(chunked)), body:
<?xml version="1.0" encoding="UTF-8"?>
<Response>
  <Errors>
    <Error>
      <Code>InvalidParameterCombination</Code>
      <Message>The parameter instancesSet cannot be used with the parameter maxResults</Message>
    </Error>
  </Errors>
  <RequestID>redacted</RequestID>
</Response>

I worked around this by removing MaxResults from the model using a preprocessor:

override def transform(ctx: TransformContext): Model =
  ctx.getTransformer.filterShapes(
    ctx.getModel,
    !_.toShapeId.equals(ShapeId.from("com.amazonaws.ec2#DescribeInstancesRequest$MaxResults"))
  )

[bpholt]

I'm hoping #1315 may resolve this, but I'm not 100% sure whether it's the same issue or not.

[Baccata]

It's a problem in the encoder derivation for UrlForm (which is the format that EC2 uses for request bodies) :

smithy4s/modules/core/src/smithy4s/http/internals/UrlFormDataEncoderSchemaVisitor.scala

Lines 139 to 141 in 4b7a8b1

	compile(field.schema)
	.contramap(field.get)
	.prepend(getKey(field.hints, field.label))

the use of contramap is incorrect here. Instead, field.getUnlessDefault should be used and pattern-matched upon to either return Nil or invoke the computed encoder for the underlying type.

[bpholt]

Something like this?

new UrlFormDataEncoder[S] {
  override def encode(value: S): List[UrlForm.FormData] =
    field.getUnlessDefault(value) match {
      case Some(a) => compile(field.schema).encode(a)
      case None    => List.empty
    }
}
  .prepend(getKey(field.hints, field.label))

[Baccata]

Nearly : compile(field.schema) should not be called on the value path, as it's an expensive operation. It should be called "statically" (wrt the value) and assigned to a val under the UrlFormDataEncoder object.

If you're intending of PR'ing the fix, you'll also have to add a unit test or two.