Skip to content

Commit

Permalink
Breaking change: Remove the deprecated always_print_primitive_fields …
Browse files Browse the repository at this point in the history 
…option from Java, Python and C++ JSON parsers.

The replacement always_print_without_presence_fields should be used instead, which is very similar but has consistent handling of optional fields by not affecting them.

PiperOrigin-RevId: 604381178
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Feb 5, 2024
1 parent 4c61b43 commit 2699579
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 643 deletions.
28 changes: 0 additions & 28 deletions java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,34 +178,6 @@ public Printer usingTypeRegistry(com.google.protobuf.TypeRegistry registry) {
sortingMapKeys);
}

/**
* Creates a new {@link Printer} that will always print fields unless they are a message type or
* in a oneof.
*
* <p>Note that this does print Proto2 Optional but does not print Proto3 Optional fields, as
* the latter is represented using a synthetic oneof.
*
* <p>The new Printer clones all other configurations from the current {@link Printer}.
*
* @deprecated Prefer {@link #alwaysPrintFieldsWithNoPresence}
*/
@Deprecated
public Printer includingDefaultValueFields() {
if (shouldPrintDefaults != ShouldPrintDefaults.ONLY_IF_PRESENT) {
throw new IllegalStateException(
"JsonFormat includingDefaultValueFields has already been set.");
}
return new Printer(
registry,
oldRegistry,
ShouldPrintDefaults.ALWAYS_PRINT_EXCEPT_MESSAGES_AND_ONEOFS,
ImmutableSet.of(),
preservingProtoFieldNames,
omittingInsignificantWhitespace,
printingEnumsAsInts,
sortingMapKeys);
}

/**
* Creates a new {@link Printer} that will also print default-valued fields if their
* FieldDescriptors are found in the supplied set. Empty repeated fields and map fields will be

This comment has been minimized.

Sign in to view

Copy link
@arklian

arklian Jul 28, 2024

The comment for includingDefaultValueFields(Set<FieldDescriptor> fieldsToAlwaysOutput) is no longer correct, now that the function without args doesn't exist.
Expand Down
302 changes: 0 additions & 302 deletions java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1527,312 +1527,10 @@ public void testDefaultValueOptionsProto3() throws Exception {
+ " \"repeatedRecursive\": []\n"
+ "}";

// includingDefaultValueFields() and alwaysPrintFieldsWithNoPresence() should
// behave identically on the proto3 test message:
assertThat(JsonFormat.printer().includingDefaultValueFields().print(message))
.isEqualTo(expectedJsonWithDefaults);
assertThat(JsonFormat.printer().alwaysPrintFieldsWithNoPresence().print(message))
.isEqualTo(expectedJsonWithDefaults);
}

@Test
public void testDefaultValueForSpecificFieldsOptionProto3() throws Exception {
TestAllTypes message = TestAllTypes.getDefaultInstance();
Set<FieldDescriptor> fixedFields = new HashSet<>();
for (FieldDescriptor fieldDesc : TestAllTypes.getDescriptor().getFields()) {
if (fieldDesc.getName().contains("_fixed")) {
fixedFields.add(fieldDesc);
}
}

assertThat(JsonFormat.printer().includingDefaultValueFields(fixedFields).print(message))
.isEqualTo(
"{\n"
+ " \"optionalFixed32\": 0,\n"
+ " \"optionalFixed64\": \"0\",\n"
+ " \"repeatedFixed32\": [],\n"
+ " \"repeatedFixed64\": []\n"
+ "}");

TestAllTypes messageNonDefaults =
message.toBuilder().setOptionalInt64(1234).setOptionalFixed32(3232).build();
assertThat(
JsonFormat.printer().includingDefaultValueFields(fixedFields).print(messageNonDefaults))
.isEqualTo(
"{\n"
+ " \"optionalInt64\": \"1234\",\n"
+ " \"optionalFixed32\": 3232,\n"
+ " \"optionalFixed64\": \"0\",\n"
+ " \"repeatedFixed32\": [],\n"
+ " \"repeatedFixed64\": []\n"
+ "}");
}

@Test
public void testDefaultValueForSpecificFieldsProto3_doesntHonorMessageFields() throws Exception {
TestAllTypes message = TestAllTypes.getDefaultInstance();
Set<FieldDescriptor> fixedFields =
ImmutableSet.of(
TestAllTypes.getDescriptor().findFieldByName("optional_bool"),
TestAllTypes.getDescriptor().findFieldByName("optional_recursive"));

assertThat(JsonFormat.printer().includingDefaultValueFields(fixedFields).print(message))
.isEqualTo("{\n \"optionalBool\": false\n}");
}

@Test
public void testRejectChangingDefaultFieldOptionMultipleTimes() throws Exception {
Set<FieldDescriptor> fixedFields = new HashSet<>();
for (FieldDescriptor fieldDesc : TestAllTypes.getDescriptor().getFields()) {
if (fieldDesc.getName().contains("_fixed")) {
fixedFields.add(fieldDesc);
}
}
try {
JsonFormat.printer().includingDefaultValueFields().includingDefaultValueFields();
assertWithMessage("IllegalStateException is expected.").fail();
} catch (IllegalStateException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}

try {
JsonFormat.printer().includingDefaultValueFields().includingDefaultValueFields(fixedFields);
assertWithMessage("IllegalStateException is expected.").fail();
} catch (IllegalStateException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}

try {
JsonFormat.printer().includingDefaultValueFields(fixedFields).includingDefaultValueFields();
assertWithMessage("IllegalStateException is expected.").fail();
} catch (IllegalStateException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}

try {
JsonFormat.printer()
.includingDefaultValueFields(fixedFields)
.includingDefaultValueFields(fixedFields);
assertWithMessage("IllegalStateException is expected.").fail();
} catch (IllegalStateException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}

Set<FieldDescriptor> intFields = new HashSet<>();
for (FieldDescriptor fieldDesc : TestAllTypes.getDescriptor().getFields()) {
if (fieldDesc.getName().contains("_int")) {
intFields.add(fieldDesc);
}
}

try {
JsonFormat.printer()
.includingDefaultValueFields(intFields)
.includingDefaultValueFields(fixedFields);
assertWithMessage("IllegalStateException is expected.").fail();
} catch (IllegalStateException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}

try {
JsonFormat.printer().includingDefaultValueFields(null);
assertWithMessage("IllegalArgumentException is expected.").fail();
} catch (IllegalArgumentException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}

try {
JsonFormat.printer().includingDefaultValueFields(Collections.<FieldDescriptor>emptySet());
assertWithMessage("IllegalArgumentException is expected.").fail();
} catch (IllegalArgumentException e) {
// Expected.
assertWithMessage("Exception message should mention includingDefaultValueFields.")
.that(e.getMessage().contains("includingDefaultValueFields"))
.isTrue();
}
}

@Test
public void testDefaultValuesOptionProto3Maps() throws Exception {
TestMap mapMessage = TestMap.getDefaultInstance();
assertThat(JsonFormat.printer().print(mapMessage)).isEqualTo("{\n}");
assertThat(JsonFormat.printer().includingDefaultValueFields().print(mapMessage))
.isEqualTo(
"{\n"
+ " \"int32ToInt32Map\": {\n"
+ " },\n"
+ " \"int64ToInt32Map\": {\n"
+ " },\n"
+ " \"uint32ToInt32Map\": {\n"
+ " },\n"
+ " \"uint64ToInt32Map\": {\n"
+ " },\n"
+ " \"sint32ToInt32Map\": {\n"
+ " },\n"
+ " \"sint64ToInt32Map\": {\n"
+ " },\n"
+ " \"fixed32ToInt32Map\": {\n"
+ " },\n"
+ " \"fixed64ToInt32Map\": {\n"
+ " },\n"
+ " \"sfixed32ToInt32Map\": {\n"
+ " },\n"
+ " \"sfixed64ToInt32Map\": {\n"
+ " },\n"
+ " \"boolToInt32Map\": {\n"
+ " },\n"
+ " \"stringToInt32Map\": {\n"
+ " },\n"
+ " \"int32ToInt64Map\": {\n"
+ " },\n"
+ " \"int32ToUint32Map\": {\n"
+ " },\n"
+ " \"int32ToUint64Map\": {\n"
+ " },\n"
+ " \"int32ToSint32Map\": {\n"
+ " },\n"
+ " \"int32ToSint64Map\": {\n"
+ " },\n"
+ " \"int32ToFixed32Map\": {\n"
+ " },\n"
+ " \"int32ToFixed64Map\": {\n"
+ " },\n"
+ " \"int32ToSfixed32Map\": {\n"
+ " },\n"
+ " \"int32ToSfixed64Map\": {\n"
+ " },\n"
+ " \"int32ToFloatMap\": {\n"
+ " },\n"
+ " \"int32ToDoubleMap\": {\n"
+ " },\n"
+ " \"int32ToBoolMap\": {\n"
+ " },\n"
+ " \"int32ToStringMap\": {\n"
+ " },\n"
+ " \"int32ToBytesMap\": {\n"
+ " },\n"
+ " \"int32ToMessageMap\": {\n"
+ " },\n"
+ " \"int32ToEnumMap\": {\n"
+ " }\n"
+ "}");
}

@Test
public void testDefaultValueOptionsProto3Oneofs() throws Exception {
TestOneof oneofMessage = TestOneof.getDefaultInstance();
assertThat(JsonFormat.printer().print(oneofMessage)).isEqualTo("{\n}");
assertThat(JsonFormat.printer().includingDefaultValueFields().print(oneofMessage))
.isEqualTo("{\n}");
assertThat(JsonFormat.printer().alwaysPrintFieldsWithNoPresence().print(oneofMessage))
.isEqualTo("{\n}");

oneofMessage = TestOneof.newBuilder().setOneofInt32(42).build();
assertThat(JsonFormat.printer().print(oneofMessage)).isEqualTo("{\n \"oneofInt32\": 42\n}");
assertThat(JsonFormat.printer().includingDefaultValueFields().print(oneofMessage))
.isEqualTo("{\n \"oneofInt32\": 42\n}");
assertThat(JsonFormat.printer().alwaysPrintFieldsWithNoPresence().print(oneofMessage))
.isEqualTo("{\n \"oneofInt32\": 42\n}");

TestOneof.Builder oneofBuilder = TestOneof.newBuilder();
mergeFromJson("{\n" + " \"oneofNullValue\": null \n" + "}", oneofBuilder);
oneofMessage = oneofBuilder.build();
assertThat(JsonFormat.printer().print(oneofMessage))
.isEqualTo("{\n \"oneofNullValue\": null\n}");
assertThat(JsonFormat.printer().includingDefaultValueFields().print(oneofMessage))
.isEqualTo("{\n \"oneofNullValue\": null\n}");
assertThat(JsonFormat.printer().alwaysPrintFieldsWithNoPresence().print(oneofMessage))
.isEqualTo("{\n \"oneofNullValue\": null\n}");
}

@Test
public void testIncludingDefaultValueOptionsWithProto2Optional() throws Exception {
TestAllTypesProto2 message = TestAllTypesProto2.getDefaultInstance();
assertThat(JsonFormat.printer().print(message)).isEqualTo("{\n}");
// includingDefaultValueFields() and alwaysPrintFieldsWithNoPresence()
// behave differently on a proto2 message: the former includes the proto2 explicit presence
// fields and the latter does not.
assertThat(JsonFormat.printer().includingDefaultValueFields().print(message))
.isEqualTo(
"{\n"
+ " \"optionalInt32\": 0,\n"
+ " \"optionalInt64\": \"0\",\n"
+ " \"optionalUint32\": 0,\n"
+ " \"optionalUint64\": \"0\",\n"
+ " \"optionalSint32\": 0,\n"
+ " \"optionalSint64\": \"0\",\n"
+ " \"optionalFixed32\": 0,\n"
+ " \"optionalFixed64\": \"0\",\n"
+ " \"optionalSfixed32\": 0,\n"
+ " \"optionalSfixed64\": \"0\",\n"
+ " \"optionalFloat\": 0.0,\n"
+ " \"optionalDouble\": 0.0,\n"
+ " \"optionalBool\": false,\n"
+ " \"optionalString\": \"\",\n"
+ " \"optionalBytes\": \"\",\n"
+ " \"optionalNestedEnum\": \"FOO\",\n"
+ " \"repeatedInt32\": [],\n"
+ " \"repeatedInt64\": [],\n"
+ " \"repeatedUint32\": [],\n"
+ " \"repeatedUint64\": [],\n"
+ " \"repeatedSint32\": [],\n"
+ " \"repeatedSint64\": [],\n"
+ " \"repeatedFixed32\": [],\n"
+ " \"repeatedFixed64\": [],\n"
+ " \"repeatedSfixed32\": [],\n"
+ " \"repeatedSfixed64\": [],\n"
+ " \"repeatedFloat\": [],\n"
+ " \"repeatedDouble\": [],\n"
+ " \"repeatedBool\": [],\n"
+ " \"repeatedString\": [],\n"
+ " \"repeatedBytes\": [],\n"
+ " \"repeatedNestedMessage\": [],\n"
+ " \"repeatedNestedEnum\": [],\n"
+ " \"optionalAliasedEnum\": \"ALIAS_FOO\",\n"
+ " \"repeatedRecursive\": []\n"
+ "}");
assertThat(JsonFormat.printer().alwaysPrintFieldsWithNoPresence().print(message))
.isEqualTo(
"{\n"
+ " \"repeatedInt32\": [],\n"
+ " \"repeatedInt64\": [],\n"
+ " \"repeatedUint32\": [],\n"
+ " \"repeatedUint64\": [],\n"
+ " \"repeatedSint32\": [],\n"
+ " \"repeatedSint64\": [],\n"
+ " \"repeatedFixed32\": [],\n"
+ " \"repeatedFixed64\": [],\n"
+ " \"repeatedSfixed32\": [],\n"
+ " \"repeatedSfixed64\": [],\n"
+ " \"repeatedFloat\": [],\n"
+ " \"repeatedDouble\": [],\n"
+ " \"repeatedBool\": [],\n"
+ " \"repeatedString\": [],\n"
+ " \"repeatedBytes\": [],\n"
+ " \"repeatedNestedMessage\": [],\n"
+ " \"repeatedNestedEnum\": [],\n"
+ " \"repeatedRecursive\": []\n"
+ "}");
}

@Test
public void testDefaultValueForSpecificFieldsOptionProto2() throws Exception {
TestAllTypesProto2 message = TestAllTypesProto2.getDefaultInstance();
Expand Down
Loading

0 comments on commit 2699579

Please sign in to comment.