Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,9 @@ public <T> void readStruct(Schema schema, T state, StructMemberConsumer<T> struc
try {
var fieldToMember = settings.fieldMapper().fieldToMember(schema);
for (var memberName = parser.nextName(); memberName != null; memberName = parser.nextName()) {
if (parser.nextToken() != VALUE_NULL) {
var member = fieldToMember.member(memberName);
var member = fieldToMember.member(memberName);
if (parser.nextToken() != VALUE_NULL
|| (member != null && structMemberConsumer.supportsNullValues(member))) {
if (member != null) {
structMemberConsumer.accept(state, member, this);
} else if (schema.type() == ShapeType.STRUCTURE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,8 @@ public <T> void readStruct(Schema schema, T state, StructMemberConsumer<T> struc
&& p + 4 <= localEnd
&& localBuf[p + 1] == 'u'
&& localBuf[p + 2] == 'l'
&& localBuf[p + 3] == 'l') {
&& localBuf[p + 3] == 'l'
&& !structMemberConsumer.supportsNullValues(member)) {
p += 4;
} else {
// Write pos back before callback, reload after
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,64 @@ public void nullMemberValueSkippedInStruct(JsonSerdeProvider provider) {
}
}

@PerProvider
public void nullMemberValuePassedToConsumerWhenSupported(JsonSerdeProvider provider) {
try (var codec = codecBuilder(provider).useJsonName(true).build()) {
var de = codec.createDeserializer(
"{\"name\":null,\"Color\":\"red\"}".getBytes(StandardCharsets.UTF_8));
Set<String> members = new LinkedHashSet<>();
List<Boolean> nullFlags = new ArrayList<>();
de.readStruct(JsonTestData.BIRD, members, new ShapeDeserializer.StructMemberConsumer<>() {
@Override
public void accept(Set<String> state, Schema member, ShapeDeserializer deser) {
state.add(member.memberName());
nullFlags.add(deser.isNull());
if (deser.isNull()) {
deser.readNull();
} else {
deser.readString(member);
}
}

@Override
public boolean supportsNullValues(Schema memberSchema) {
return true;
}
});
// "name" was null but consumer was still called because supportsNullValues is true
assertThat(members, contains("name", "color"));
assertThat(nullFlags, contains(true, false));
}
}

@PerProvider
public void nullMemberValueSkippedWhenNotSupportedBySpecificMember(JsonSerdeProvider provider) {
try (var codec = codecBuilder(provider).useJsonName(true).build()) {
var de = codec.createDeserializer(
"{\"name\":null,\"Color\":null,\"nested\":\"hi\"}".getBytes(StandardCharsets.UTF_8));
Set<String> members = new LinkedHashSet<>();
de.readStruct(JsonTestData.BIRD, members, new ShapeDeserializer.StructMemberConsumer<>() {
@Override
public void accept(Set<String> state, Schema member, ShapeDeserializer deser) {
state.add(member.memberName());
if (deser.isNull()) {
deser.readNull();
} else {
deser.readString(member);
}
}

@Override
public boolean supportsNullValues(Schema memberSchema) {
// Only support null for "name", not for "color"
return memberSchema.memberName().equals("name");
}
});
// "name" null is passed through, "color" null is skipped
assertThat(members, contains("name", "nested"));
}
}

@ParameterizedTest
@MethodSource("smithyOnly")
public void rejectsNonObjectForMap(JsonSerdeProvider provider) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,14 @@ public final class SymbolProperties {
*/
public static final Property<Boolean> IS_NULLABLE = Property.named("is-nullable");

/**
* Indicates if a member supports explicit null values during deserialization.
*
* <p>When set to {@code true}, the generated deserializer will pass null values through to the
* struct member consumer rather than skipping them, allowing the member to distinguish between
* absent and explicitly null.
*/
public static final Property<Boolean> SUPPORTS_NULL_VALUES = Property.named("supports-null-values");

private SymbolProperties() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
import java.util.Map;
import software.amazon.smithy.codegen.core.SymbolProvider;
import software.amazon.smithy.java.codegen.CodegenUtils;
import software.amazon.smithy.java.codegen.SymbolProperties;
import software.amazon.smithy.java.codegen.writer.JavaWriter;
import software.amazon.smithy.java.core.schema.Schema;
import software.amazon.smithy.java.core.serde.ShapeDeserializer;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.traits.ErrorTrait;
Expand Down Expand Up @@ -56,7 +58,15 @@ public void accept(Builder builder, ${sdkSchema:T} member, ${shapeDeserializer:T
@Override
public void unknownMember(Builder builder, ${string:T} memberName) {
builder.$$unknownMember(memberName);
}${/union}
}${/union}${?supportsNullValues}

@Override
public boolean supportsNullValues(${sdkSchema:T} member) {
return switch (member.memberIndex()) {
${nullValueCases:C|}
default -> false;
};
}${/supportsNullValues}
}""";
writer.putContext("shapeDeserializer", ShapeDeserializer.class);
writer.putContext("sdkSchema", Schema.class);
Expand All @@ -66,6 +76,8 @@ public void unknownMember(Builder builder, ${string:T} memberName) {
writer.putContext("union", shape.isUnionShape());
writer.putContext("illegalArg", IllegalArgumentException.class);
writer.putContext("isError", shape.hasTrait(ErrorTrait.class));
writer.putContext("supportsNullValues", hasMembersThatSupportsNullValues(shape));
writer.putContext("nullValueCases", writer.consumer(this::generateMemberNullValuesSwitchCases));
writer.write(template);
writer.popState();
}
Expand All @@ -83,4 +95,31 @@ private void generateMemberSwitchCases(JavaWriter writer) {
writer.popState();
}
}

private void generateMemberNullValuesSwitchCases(JavaWriter writer) {
int idx = 0;
for (var iter = CodegenUtils.getSortedMembers(shape).iterator(); iter.hasNext(); idx++) {
var member = iter.next();

if (memberSupportsNullValues(member)) {
writer.write("case $L -> true;", idx);
}
}
}

private boolean hasMembersThatSupportsNullValues(Shape shape) {
return shape.getAllMembers()
.values()
.stream()
.anyMatch(this::memberSupportsNullValues);
}

private boolean memberSupportsNullValues(MemberShape member) {
var memberSymbol = symbolProvider.toSymbol(member);
if (memberSymbol == null) {
return false;
}

return memberSymbol.getProperty(SymbolProperties.SUPPORTS_NULL_VALUES).orElse(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,20 @@ interface StructMemberConsumer<T> {
void accept(T state, Schema memberSchema, ShapeDeserializer memberDeserializer);

default void unknownMember(T state, String memberName) {}

/**
* Returns whether a given member supports explicit null values during deserialization.
*
* <p>When this returns {@code true}, the deserializer will invoke {@link #accept} for the member
* even when the serialized value is null, allowing the consumer to distinguish between an absent
* field and an explicitly null value.
*
* @param memberSchema Schema of the member to check.
* @return {@code true} if the member should receive null values, {@code false} to skip them.
*/
default boolean supportsNullValues(Schema memberSchema) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a property of generic serde, or is it a property of a protocol?

@timocov timocov Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I assume your question is more of "should a structure should tell if it supports null vslues or the protocol enables it for every field regardless"? I can see it from both sides, and tbh I'm still not sure that a protocol matters in this case - it just tells that "you should differentiate between passed nulls and not passed values", but I can see it being used in some places even without specifying the protocol explicitly if a business logic wants to see the difference.

It does feel like more of a protocol feature to "turn it on", but whether a field supprts such differentiation should lay down to the business logic (i.e. model). If we make it a protocol feature, then generated serde logic in shapes will become protocol-aware (currently it will fail if the serde level will call deserialising of a structure on with null value), which goes against the main principles of Smithy.

return false;
}
}

/**
Expand Down
Loading