Skip to content

Commit d75f15f

Browse files
committed
Fix incorrect bitmask exposed by DeferredMemberSchema
The bitmask applies to the member, not its target schema -- delegating to the target schema exposes the wrong bitmask. Also fix an incidental similar issue in requiredStructureMemberBitField(), though I haven't seen any negative effects from the previous implementation of that one.
1 parent fe2c41f commit d75f15f

File tree

3 files changed

+47
-3
lines changed

3 files changed

+47
-3
lines changed

core/src/main/java/software/amazon/smithy/java/core/schema/DeferredMemberSchema.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ final class DeferredMemberSchema extends Schema {
1515

1616
private final SchemaBuilder target;
1717
private final TraitMap directTraits;
18+
private final long requiredByValidationBitmask;
1819

1920
DeferredMemberSchema(MemberSchemaBuilder builder) {
2021
super(builder);
2122
this.target = builder.targetBuilder;
2223
this.directTraits = builder.directTraits;
24+
this.requiredByValidationBitmask = builder.requiredByValidationBitmask;
2325
}
2426

2527
@Override
@@ -44,12 +46,21 @@ public int requiredMemberCount() {
4446

4547
@Override
4648
long requiredByValidationBitmask() {
47-
return memberTarget().requiredByValidationBitmask();
49+
// The bitmask applies to the member itself, not its target schema, so this should not be forwarded
50+
// to memberTarget as the previous accessors do.
51+
return requiredByValidationBitmask;
4852
}
4953

5054
@Override
5155
long requiredStructureMemberBitfield() {
52-
return memberTarget().requiredStructureMemberBitfield();
56+
// MemberSchema computes this at initialization time, but here we need to defer it
57+
// until we can build the concrete target.
58+
var target = memberTarget();
59+
return SchemaBuilder.computeRequiredBitField(
60+
type(),
61+
target.requiredMemberCount(),
62+
target.members(),
63+
Schema::requiredByValidationBitmask);
5364
}
5465

5566
@Override

core/src/main/java/software/amazon/smithy/java/core/schema/MemberSchema.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final class MemberSchema extends Schema {
2626
this.requiredStructureMemberBitfield = SchemaBuilder.computeRequiredBitField(
2727
type(),
2828
target.requiredMemberCount(),
29-
builder.target.members(),
29+
target.members(),
3030
Schema::requiredByValidationBitmask);
3131
this.requiredByValidationBitmask = builder.requiredByValidationBitmask;
3232
}

core/src/test/java/software/amazon/smithy/java/core/schema/ValidatorTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,39 @@ public void detectsTooDeepRecursion() {
131131
assertThat(errors.get(0).path(), equalTo("/"));
132132
}
133133

134+
@Test
135+
public void detectsPresenceForNestedRecursiveFields() {
136+
var innerBuilder = Schema.structureBuilder(ShapeId.from("smithy.example#Inner"))
137+
.putMember("foo", PreludeSchemas.STRING, new RequiredTrait());
138+
var innerSchema = innerBuilder.build();
139+
140+
var innerInstance = TestHelper.create(innerSchema, (schema, serializer) -> {
141+
serializer.writeString(schema.member("foo"), "bar");
142+
});
143+
144+
var rootSchemaWithMember = Schema.structureBuilder(ShapeId.from("smithy.example#Root"))
145+
.putMember("inner", innerSchema, new RequiredTrait())
146+
.build();
147+
var rootInstanceWithMember = TestHelper.create(rootSchemaWithMember, (schema, serializer) -> {
148+
serializer.writeStruct(schema.member("inner"), innerInstance);
149+
});
150+
151+
var rootSchemaWithDeferredMember = Schema.structureBuilder(ShapeId.from("smithy.example#Root"))
152+
.putMember("inner", innerBuilder, new RequiredTrait())
153+
.build();
154+
var rootInstanceWithDeferredMember = TestHelper.create(rootSchemaWithDeferredMember, (schema, serializer) -> {
155+
serializer.writeStruct(schema.member("inner"), innerInstance);
156+
});
157+
158+
var validator = Validator.builder().build();
159+
160+
var memberErrors = validator.validate(rootInstanceWithMember);
161+
assertThat(memberErrors, empty());
162+
163+
var deferredMemberErrors = validator.validate(rootInstanceWithDeferredMember);
164+
assertThat(deferredMemberErrors, empty());
165+
}
166+
134167
private List<Schema> createListSchemas(int depth) {
135168
List<Schema> schemas = new ArrayList<>(depth);
136169
for (int i = depth; i > 0; i--) {

0 commit comments

Comments
 (0)