Unverified Commit 2f2e1438 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Fix serialization of required shapes (timestamps and nested shapes) (#1275)

In #1148, `@required` started being strictly interpreted by server SDKs.

That meant that when serializing values from structures, we no longer
borrow for every shape when looking into `Option`s as in:

```
if let Some(var_37) = &input.value
```

(See for example `RustWriter.serializeStructure` from
`JsonSerializerGenerator.kt` for the relevant serialization
code-generation routine)

Instead, we attempt to serialize the unborrowed required shape value.
This works well for booleans, numeric types (since they are `Copy`),
collections (since we borrow their items while iterating), and strings
(since we call `as_str()`). It also worked for unions and documents,
since we already borrowed (note that `ValueExpression.kt` makes sure to
not borrow twice if the value is already a reference, like when clients
look into `Option`s).

We fixed borrowing of blobs in #1269.

However, we currently don't borrow for timestamps and nested shapes.
This commit fixes that, and adds a comprehensive protocol test to ensure
we exercise all the lines relevant to serialization of required shapes.
parent 565d8383
Loading
Loading
Loading
Loading
+75 −3
Original line number Original line Diff line number Diff line
@@ -15,7 +15,8 @@ service MiscService {
    ],
    ],
}
}


/// To not regress on https://github.com/awslabs/smithy-rs/pull/1266
/// This operation tests that (de)serializing required values from a nested
/// shape works correctly.
@http(uri: "/operation", method: "GET")
@http(uri: "/operation", method: "GET")
operation OperationWithInnerRequiredShape {
operation OperationWithInnerRequiredShape {
    input: OperationWithInnerRequiredShapeInput,
    input: OperationWithInnerRequiredShapeInput,
@@ -26,13 +27,84 @@ structure OperationWithInnerRequiredShapeInput {
    inner: InnerShape
    inner: InnerShape
}
}


structure OperationWithInnerRequiredShapeOutput {
    inner: InnerShape
}

structure InnerShape {
structure InnerShape {
    @required
    @required
    requiredInnerMostShape: InnermostShape
    requiredInnerMostShape: InnermostShape
}
}


structure InnermostShape {
structure InnermostShape {
    aString: String
    @required
    aString: String,

    @required
    aBoolean: Boolean,

    @required
    aByte: Byte,

    @required
    aShort: Short,

    @required
    anInt: Integer,

    @required
    aLong: Long,

    @required
    aFloat: Float,

    @required
    aDouble: Double,

    // TODO(https://github.com/awslabs/smithy-rs/issues/312)
    // @required
    // aBigInteger: BigInteger,

    // @required
    // aBigDecimal: BigDecimal,

    @required
    aTimestamp: Timestamp,

    @required
    aDocument: Timestamp,

    @required
    aStringList: AStringList,

    @required
    aStringMap: AMap,

    @required
    aStringSet: AStringSet,

    @required
    aBlob: Blob,

    @required
    aUnion: AUnion
}

list AStringList {
    member: String
}
}


structure OperationWithInnerRequiredShapeOutput { }
list AStringSet {
    member: String
}

map AMap {
    key: String,
    value: Timestamp
}

union AUnion {
    i32: Integer,
    string: String,
    time: Timestamp,
}
+6 −6
Original line number Original line Diff line number Diff line
@@ -347,26 +347,26 @@ class JsonSerializerGenerator(
                )
                )
            }
            }
            is BlobShape -> rust(
            is BlobShape -> rust(
                "$writer.string_unchecked(&#T(${value.name}.as_ref()));",
                "$writer.string_unchecked(&#T(${value.asRef()}));",
                RuntimeType.Base64Encode(runtimeConfig)
                RuntimeType.Base64Encode(runtimeConfig)
            )
            )
            is TimestampShape -> {
            is TimestampShape -> {
                val timestampFormat =
                val timestampFormat =
                    httpBindingResolver.timestampFormat(context.shape, HttpLocation.DOCUMENT, EPOCH_SECONDS)
                    httpBindingResolver.timestampFormat(context.shape, HttpLocation.DOCUMENT, EPOCH_SECONDS)
                val timestampFormatType = RuntimeType.TimestampFormat(runtimeConfig, timestampFormat)
                val timestampFormatType = RuntimeType.TimestampFormat(runtimeConfig, timestampFormat)
                rust("$writer.date_time(${value.name}, #T)?;", timestampFormatType)
                rust("$writer.date_time(${value.asRef()}, #T)?;", timestampFormatType)
            }
            }
            is CollectionShape -> jsonArrayWriter(context) { arrayName ->
            is CollectionShape -> jsonArrayWriter(context) { arrayName ->
                serializeCollection(Context(arrayName, context.valueExpression, target))
                serializeCollection(Context(arrayName, value, target))
            }
            }
            is MapShape -> jsonObjectWriter(context) { objectName ->
            is MapShape -> jsonObjectWriter(context) { objectName ->
                serializeMap(Context(objectName, context.valueExpression, target))
                serializeMap(Context(objectName, value, target))
            }
            }
            is StructureShape -> jsonObjectWriter(context) { objectName ->
            is StructureShape -> jsonObjectWriter(context) { objectName ->
                serializeStructure(StructContext(objectName, context.valueExpression.name, target))
                serializeStructure(StructContext(objectName, value.asRef(), target))
            }
            }
            is UnionShape -> jsonObjectWriter(context) { objectName ->
            is UnionShape -> jsonObjectWriter(context) { objectName ->
                serializeUnion(Context(objectName, context.valueExpression, target))
                serializeUnion(Context(objectName, value, target))
            }
            }
            is DocumentShape -> rust("$writer.document(${value.asRef()});")
            is DocumentShape -> rust("$writer.document(${value.asRef()});")
            else -> TODO(target.toString())
            else -> TODO(target.toString())