diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/utils/FieldMasks.java b/google-ads/src/main/java/com/google/ads/googleads/lib/utils/FieldMasks.java index 7eff0aa832..0f7549e82c 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/utils/FieldMasks.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/utils/FieldMasks.java @@ -17,7 +17,6 @@ import com.google.common.base.Preconditions; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.Descriptors.FieldDescriptor.Type; import com.google.protobuf.FieldMask; import com.google.protobuf.GeneratedMessageV3; import com.google.protobuf.Message; @@ -76,12 +75,14 @@ private static void compare( mask.addPaths(fieldName); } } else { + boolean hasValueChanged = + original.hasField(field) != modified.hasField(field) + || !Objects.equals(originalValue, modifiedValue); switch (field.getJavaType()) { case MESSAGE: // Because getField never returns null, we use hasField to distinguish null // from empty message when getType() == MESSAGE - if (original.hasField(field) != modified.hasField(field) - || !Objects.equals(originalValue, modifiedValue)) { + if (hasValueChanged) { if (isWrapperType(field.getMessageType())) { // For wrapper types, just emit the field name. mask.addPaths(fieldName); @@ -103,7 +104,7 @@ private static void compare( case BYTE_STRING: case ENUM: // Handle all java types except MESSAGE - if (!Objects.equals(originalValue, modifiedValue)) { + if (hasValueChanged) { mask.addPaths(fieldName); } break; diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksParameterizedTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksParameterizedTest.java new file mode 100644 index 0000000000..5729fb8943 --- /dev/null +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksParameterizedTest.java @@ -0,0 +1,75 @@ +// Copyright 2018 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.ads.googleads.lib.utils; + +import com.google.ads.googleads.test.Resource; +import com.google.ads.googleads.test.TestCase; +import com.google.ads.googleads.test.TestSuite; +import com.google.common.base.Charsets; +import com.google.common.truth.Truth; +import com.google.protobuf.FieldMask; +import com.google.protobuf.TextFormat; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +/** Tests the {@link FieldMasks} utility. */ +@RunWith(Parameterized.class) +public class FieldMasksParameterizedTest { + @Parameters(name = "{index}: {0}") + public static Collection data() throws IOException { + InputStream stream = + FieldMasksParameterizedTest.class.getResourceAsStream("/testdata/test_cases.textproto"); + Readable readable = new InputStreamReader(stream, Charsets.UTF_8); + + TestSuite.Builder builder = TestSuite.newBuilder(); + TextFormat.merge(readable, builder); + TestSuite testSuite = builder.build(); + + List result = new ArrayList<>(); + for (TestCase testCase : testSuite.getTestCasesList()) { + result.add(new Object[] {testCase.getDescription(), testCase}); + } + return result; + } + + private final TestCase testCase; + + public FieldMasksParameterizedTest(String description, TestCase testCase) { + this.testCase = testCase; + } + + @Test + public void testFieldMaskCompare() { + FieldMask actual = + FieldMasks.compare(testCase.getOriginalResource(), testCase.getModifiedResource()); + Truth.assertThat(testCase.getExpectedMask()).isEqualTo(actual); + } + + @Test + public void testFieldMaskAllSetFieldsOf() { + Resource resource = testCase.getModifiedResource(); + FieldMask actual = FieldMasks.allSetFieldsOf(resource); + FieldMask expected = FieldMasks.compare(resource.getDefaultInstanceForType(), resource); + Truth.assertThat(expected).isEqualTo(actual); + } +} diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksTest.java index a339475ea8..bc274d53b7 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksTest.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/utils/FieldMasksTest.java @@ -1,76 +1,24 @@ -// Copyright 2018 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - package com.google.ads.googleads.lib.utils; -import com.google.ads.googleads.test.Resource; -import com.google.ads.googleads.test.TestCase; -import com.google.ads.googleads.test.TestSuite; -import com.google.common.base.Charsets; -import com.google.common.truth.Truth; -import com.google.protobuf.FieldMask; -import com.google.protobuf.TextFormat; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; +import static org.junit.Assert.assertEquals; + +import com.google.ads.googleads.v5.resources.Campaign; +import com.google.common.collect.ImmutableList; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; +import org.junit.runners.JUnit4; -/** - * Tests the {@link FieldMasks} utility. - */ -@RunWith(Parameterized.class) +@RunWith(JUnit4.class) public class FieldMasksTest { - @Parameters(name = "{index}: {0}") - public static Collection data() throws IOException { - InputStream stream = FieldMasksTest.class.getResourceAsStream("/testdata/test_cases.textproto"); - Readable readable = new InputStreamReader(stream, Charsets.UTF_8); - - TestSuite.Builder builder = TestSuite.newBuilder(); - TextFormat.merge(readable, builder); - TestSuite testSuite = builder.build(); - - List result = new ArrayList<>(); - for (TestCase testCase : testSuite.getTestCasesList()) { - result.add(new Object[] {testCase.getDescription(), testCase}); - } - return result; - } - - private final TestCase testCase; - - public FieldMasksTest(String description, TestCase testCase) { - this.testCase = testCase; - } - - @Test - public void testFieldMaskCompare() { - FieldMask actual = - FieldMasks.compare(testCase.getOriginalResource(), testCase.getModifiedResource()); - Truth.assertThat(testCase.getExpectedMask()).isEqualTo(actual); - } + // Regression test for https://github.com/googleads/google-ads-java/issues/344. @Test - public void testFieldMaskAllSetFieldsOf() { - Resource resource = testCase.getModifiedResource(); - FieldMask actual = FieldMasks.allSetFieldsOf(resource); - FieldMask expected = FieldMasks.compare(resource.getDefaultInstanceForType(), resource); - Truth.assertThat(expected).isEqualTo(actual); + public void optionalField_withDefaultValue_detectsChange() { + Campaign.Builder builder = Campaign.newBuilder(); + builder.getManualCpcBuilder().setEnhancedCpcEnabled(false); + Campaign campaign = builder.build(); + assertEquals( + ImmutableList.of("manual_cpc.enhanced_cpc_enabled"), + FieldMasks.allSetFieldsOf(campaign).getPathsList()); } }