Merge pull request #10984 from protocolbuffers/googleberg-patch-nested-builder-fix

Fix stale cached submessages by marking nested builder as clean after clear is called
pull/11028/head
Jerry Berg 2 years ago committed by GitHub
commit 3c3e66ade1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java
  2. 22
      java/core/src/test/java/com/google/protobuf/NestedBuildersTest.java
  3. 5
      java/core/src/test/proto/com/google/protobuf/nested_builders_test.proto

@ -199,6 +199,9 @@ public class SingleFieldBuilderV3<
builder = null;
}
onChanged();
// After clearing, parent is dirty, but this field builder is now clean and any changes should
// trickle up.
isClean = true;
return this;
}

@ -33,6 +33,7 @@ package com.google.protobuf;
import static com.google.common.truth.Truth.assertThat;
import protobuf_unittest.Engine;
import protobuf_unittest.TimingBelt;
import protobuf_unittest.Vehicle;
import protobuf_unittest.Wheel;
import java.util.ArrayList;
@ -48,6 +49,27 @@ import org.junit.runners.JUnit4;
@RunWith(JUnit4.class)
public class NestedBuildersTest {
@Test
public void test3LayerPropagationWithIntermediateClear() {
Vehicle.Builder vehicleBuilder = Vehicle.newBuilder();
vehicleBuilder.getEngineBuilder().getTimingBeltBuilder();
// This step detaches the TimingBelt.Builder (though it leaves a SingleFieldBuilder in place)
vehicleBuilder.getEngineBuilder().clear();
// These steps build the middle and top level messages, it used to leave the vestigial
// TimingBelt.Builder in a state where further changes didn't propagate anymore
Object unused = vehicleBuilder.getEngineBuilder().build();
unused = vehicleBuilder.build();
TimingBelt expected = TimingBelt.newBuilder().setNumberOfTeeth(124).build();
vehicleBuilder.getEngineBuilder().setTimingBelt(expected);
// Testing that b/254158939 is fixed. It used to be that the setTimingBelt call above didn't
// propagate a change notification and the call below would return a stale version of the timing
// belt.
assertThat(vehicleBuilder.getEngine().getTimingBelt()).isEqualTo(expected);
}
@Test
public void testMessagesAndBuilders() {
Vehicle.Builder vehicleBuilder = Vehicle.newBuilder();

@ -45,6 +45,11 @@ message Vehicle {
message Engine {
optional int32 cylinder = 1;
optional int32 liters = 2;
optional TimingBelt timing_belt = 3;
}
message TimingBelt {
optional int32 number_of_teeth = 1;
}
message Wheel {

Loading…
Cancel
Save