From 25f9a5f0d0e13dae4ff7c7a8bc6fb238dc7d5b60 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:13:36 +0100 Subject: [PATCH 01/54] #300: Remove empty space. --- docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/README.md b/docs/README.md index 671fe7ce45..ce8c24b878 100644 --- a/docs/README.md +++ b/docs/README.md @@ -30,7 +30,7 @@ Currently this library supports synchronising the following entities in commerce - [InventoryEntries](usage/INVENTORY_SYNC.md) - [ProductTypes](usage/PRODUCT_TYPE_SYNC.md) - [Types](usage/TYPE_SYNC.md) - + ![commercetools-java-sync-final 001](https://user-images.githubusercontent.com/9512131/31230702-0f2255a6-a9e5-11e7-9412-04ed52641dde.png) From 76e44736532e4083b4ad7e04bdcf769694dcf568 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:14:14 +0100 Subject: [PATCH 02/54] #300: Statically import assertion utils. --- .../benchmark/ProductTypeSyncBenchmark.java | 14 ++++--------- .../sync/benchmark/TypeSyncBenchmark.java | 20 +++++++------------ 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java b/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java index b951974cf9..65d363b308 100644 --- a/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java +++ b/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java @@ -1,6 +1,5 @@ package com.commercetools.sync.benchmark; -import com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics; import com.commercetools.sync.commons.utils.SyncSolutionInfo; import com.commercetools.sync.producttypes.ProductTypeSync; import com.commercetools.sync.producttypes.ProductTypeSyncOptions; @@ -36,6 +35,7 @@ import static com.commercetools.sync.benchmark.BenchmarkUtils.UPDATES_ONLY; import static com.commercetools.sync.benchmark.BenchmarkUtils.calculateDiff; import static com.commercetools.sync.benchmark.BenchmarkUtils.saveNewResult; +import static com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics.assertThat; import static com.commercetools.sync.integration.commons.utils.ProductTypeITUtils.ATTRIBUTE_DEFINITION_DRAFT_1; import static com.commercetools.sync.integration.commons.utils.ProductTypeITUtils.deleteProductTypes; import static com.commercetools.sync.integration.commons.utils.SphereClientUtils.CTP_TARGET_CLIENT; @@ -111,9 +111,7 @@ public void sync_NewProductTypes_ShouldCreateProductTypes() throws IOException { assertThat(totalNumberOfProductTypes).isCompletedWithValue(NUMBER_OF_RESOURCE_UNDER_TEST); - AssertionsForStatistics - .assertThat(syncStatistics) - .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, NUMBER_OF_RESOURCE_UNDER_TEST, 0, 0); + assertThat(syncStatistics).hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, NUMBER_OF_RESOURCE_UNDER_TEST, 0, 0); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); assertThat(warningCallBackMessages).isEmpty(); @@ -172,9 +170,7 @@ public void sync_ExistingProductTypes_ShouldUpdateProductTypes() throws IOExcept assertThat(totalNumberOfProductTypes).isCompletedWithValue(NUMBER_OF_RESOURCE_UNDER_TEST); // Assert statistics - AssertionsForStatistics - .assertThat(syncStatistics) - .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, 0, NUMBER_OF_RESOURCE_UNDER_TEST, 0); + assertThat(syncStatistics).hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, 0, NUMBER_OF_RESOURCE_UNDER_TEST, 0); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); @@ -235,9 +231,7 @@ public void sync_WithSomeExistingProductTypes_ShouldSyncProductTypes() throws IO assertThat(totalNumberOfProductTypes).isCompletedWithValue(NUMBER_OF_RESOURCE_UNDER_TEST); // Assert statistics - AssertionsForStatistics - .assertThat(syncStatistics) - .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, halfNumberOfDrafts, halfNumberOfDrafts, 0); + assertThat(syncStatistics).hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, halfNumberOfDrafts, halfNumberOfDrafts, 0); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); diff --git a/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java b/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java index be2dbadf5c..23400b64d7 100644 --- a/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java +++ b/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java @@ -1,6 +1,5 @@ package com.commercetools.sync.benchmark; -import com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics; import com.commercetools.sync.commons.utils.SyncSolutionInfo; import com.commercetools.sync.types.TypeSync; import com.commercetools.sync.types.TypeSyncOptions; @@ -38,6 +37,7 @@ import static com.commercetools.sync.benchmark.BenchmarkUtils.UPDATES_ONLY; import static com.commercetools.sync.benchmark.BenchmarkUtils.calculateDiff; import static com.commercetools.sync.benchmark.BenchmarkUtils.saveNewResult; +import static com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics.assertThat; import static com.commercetools.sync.integration.commons.utils.ITUtils.deleteTypesFromTargetAndSource; import static com.commercetools.sync.integration.commons.utils.SphereClientUtils.CTP_TARGET_CLIENT; import static com.commercetools.sync.integration.commons.utils.TypeITUtils.FIELD_DEFINITION_1; @@ -120,9 +120,7 @@ public void sync_NewTypes_ShouldCreateTypes() throws IOException { assertThat(totalNumberOfTypes).isCompletedWithValue(NUMBER_OF_RESOURCE_UNDER_TEST); - AssertionsForStatistics - .assertThat(syncStatistics) - .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, NUMBER_OF_RESOURCE_UNDER_TEST, 0, 0); + assertThat(syncStatistics).hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, NUMBER_OF_RESOURCE_UNDER_TEST, 0, 0); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); assertThat(warningCallBackMessages).isEmpty(); @@ -180,9 +178,7 @@ public void sync_ExistingTypes_ShouldUpdateTypes() throws IOException { assertThat(totalNumberOfTypes).isCompletedWithValue(NUMBER_OF_RESOURCE_UNDER_TEST); // Assert statistics - AssertionsForStatistics - .assertThat(syncStatistics) - .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, 0, NUMBER_OF_RESOURCE_UNDER_TEST, 0); + assertThat(syncStatistics).hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, 0, NUMBER_OF_RESOURCE_UNDER_TEST, 0); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); @@ -222,7 +218,7 @@ public void sync_WithSomeExistingTypes_ShouldSyncTypes() throws IOException { .isLessThanOrEqualTo(THRESHOLD); // Assert actual state of CTP project (number of updated types) - final CompletableFuture totalNumberOfUpdatedTypesWithOldFielDefinitionName = + final CompletableFuture totalNumberOfUpdatedTypesWithOldFieldDefinitionName = CTP_TARGET_CLIENT.execute(TypeQuery.of() .withPredicates(p -> p.fieldDefinitions().name().is( FIELD_DEFINITION_NAME_1 + "_old"))) @@ -230,8 +226,8 @@ public void sync_WithSomeExistingTypes_ShouldSyncTypes() throws IOException { .thenApply(Long::intValue) .toCompletableFuture(); - executeBlocking(totalNumberOfUpdatedTypesWithOldFielDefinitionName); - assertThat(totalNumberOfUpdatedTypesWithOldFielDefinitionName).isCompletedWithValue(0); + executeBlocking(totalNumberOfUpdatedTypesWithOldFieldDefinitionName); + assertThat(totalNumberOfUpdatedTypesWithOldFieldDefinitionName).isCompletedWithValue(0); // Assert actual state of CTP project (total number of existing types) final CompletableFuture totalNumberOfTypes = @@ -243,9 +239,7 @@ public void sync_WithSomeExistingTypes_ShouldSyncTypes() throws IOException { assertThat(totalNumberOfTypes).isCompletedWithValue(NUMBER_OF_RESOURCE_UNDER_TEST); // Assert statistics - AssertionsForStatistics - .assertThat(syncStatistics) - .hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, halfNumberOfDrafts, halfNumberOfDrafts, 0); + assertThat(syncStatistics).hasValues(NUMBER_OF_RESOURCE_UNDER_TEST, halfNumberOfDrafts, halfNumberOfDrafts, 0); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); From ddf7e89255e8f9140ac14721d8a4fc591c449a78 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:17:56 +0100 Subject: [PATCH 03/54] #300: Remove release notes for non exposed methods. --- docs/RELEASE_NOTES.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/RELEASE_NOTES.md b/docs/RELEASE_NOTES.md index ced7211dd6..4ebf20b402 100644 --- a/docs/RELEASE_NOTES.md +++ b/docs/RELEASE_NOTES.md @@ -43,8 +43,6 @@ - **Type Sync** - Exposed `TypeUpdateActionUtils` which contains utils for calculating needed update actions after comparing individual fields of a `Type` and a `TypeDraft`. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - **Type Sync** - Exposed `LocalizedEnumValueUpdateActionUtils` which contains utils for calculating needed update actions after comparing two lists of `LocalizedEnumValue`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - **Type Sync** - Exposed `PlainEnumValueUpdateActionUtils` which contains utils for calculating needed update actions after comparing two lists of `EnumValue`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - - **Type Sync** - Exposed `FieldDefinitionsUpdateActionUtils` which contains utils for calculating needed update actions after comparing a list of `FieldDefinition`s and a list of `FieldDefinition`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - - **Type Sync** - Exposed `FieldDefinitionUpdateActionUtils` which contains utils for calculating needed update actions after comparing a `FieldDefinition` and a `FieldDefinition`. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - **Commons** - Exposed `EnumValuesUpdateActionUtils` which contains generic utilities for calculating needed update actions after comparing two lists of `EnumValue`s or `LocalizedEnumValue`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - 📋 **Documentation** (4) From 291c5dcac0e540b0f48510092023278452dd9566 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:18:56 +0100 Subject: [PATCH 04/54] #300: Reword release note entries. --- docs/RELEASE_NOTES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/RELEASE_NOTES.md b/docs/RELEASE_NOTES.md index 4ebf20b402..06436efe16 100644 --- a/docs/RELEASE_NOTES.md +++ b/docs/RELEASE_NOTES.md @@ -58,7 +58,7 @@ - **Product Sync** - Reference keys are not validated if they are in UUID format anymore. [#166](https://github.com/commercetools/commercetools-sync-java/issues/166) - **Category Sync** - Reference keys are not validated if they are in UUID format anymore. [#166](https://github.com/commercetools/commercetools-sync-java/issues/166) - **Inventory Sync** - Reference keys are not validated if they are in UUID format anymore. [#166](https://github.com/commercetools/commercetools-sync-java/issues/166) - - **ProductType Sync** - Added `ProductTypeSyncBenchmark` to benchmark the product type sync, to be able to compare the performance of the sync with the future releases. [#301](https://github.com/commercetools/commercetools-sync-java/issues/301) + - **ProductType Sync** - Added benchmarks for the `productType` sync to be able to compare the performance of the sync with the future releases. [#301](https://github.com/commercetools/commercetools-sync-java/issues/301) - **Commons** - Bumped commercetools-jvm-sdk to version [1.37.0](http://commercetools.github.io/commercetools-jvm-sdk/apidocs/io/sphere/sdk/meta/ReleaseNotes.html#v1_37_0). - **Commons** - Bumped `mockito` to 2.23.4. - **Commons** - Bumped `com.adarshr.test-logger` to 1.6.0. @@ -69,7 +69,7 @@ - **Commons** - Bumped `com.adarshr.test-logger` to 1.6.0. - **Commons** - Bumped `org.ajoberstar.grgit` to 3.0.0. - **Commons** - Bumped gradle to version [gradle-5.0](https://docs.gradle.org/5.0/release-notes.html) - - **Type Sync** - Added `TypeSyncBenchmark` to benchmark the type sync, to be able to compare the performance of the sync with the future releases. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) + - **Type Sync** - Added benchmarks for the `type` sync to be able to compare the performance of the sync with the future releases. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - 🚧 **Breaking Changes** (11) - **Product Sync** - `allowUuid` option is now removed. [#166](https://github.com/commercetools/commercetools-sync-java/issues/166) From c87687bd3ffac25ddf45514801e1f11ed4d3342c Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:34:23 +0100 Subject: [PATCH 05/54] #300: Fix bug in calculating diff of avg in benchmarks. --- .../commercetools/sync/benchmark/ProductSyncBenchmark.java | 4 ++-- .../sync/benchmark/ProductTypeSyncBenchmark.java | 6 ++++-- .../com/commercetools/sync/benchmark/TypeSyncBenchmark.java | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/benchmark/java/com/commercetools/sync/benchmark/ProductSyncBenchmark.java b/src/benchmark/java/com/commercetools/sync/benchmark/ProductSyncBenchmark.java index 3c25cc6b3a..d58b154363 100644 --- a/src/benchmark/java/com/commercetools/sync/benchmark/ProductSyncBenchmark.java +++ b/src/benchmark/java/com/commercetools/sync/benchmark/ProductSyncBenchmark.java @@ -158,7 +158,7 @@ public void sync_ExistingProducts_ShouldUpdateProducts() throws IOException { // Calculate time taken for benchmark and assert it lies within threshold - final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_SYNC, CREATES_ONLY, totalTime); + final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_SYNC, UPDATES_ONLY, totalTime); assertThat(diff) .withFailMessage(format("Diff of benchmark '%e' is longer than expected threshold of '%d'.", diff, THRESHOLD)) @@ -222,7 +222,7 @@ public void sync_WithSomeExistingProducts_ShouldSyncProducts() throws IOExceptio // Calculate time taken for benchmark and assert it lies within threshold - final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_SYNC, CREATES_ONLY, totalTime); + final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_SYNC, CREATES_AND_UPDATES, totalTime); assertThat(diff) .withFailMessage(format("Diff of benchmark '%e' is longer than expected threshold of '%d'.", diff, THRESHOLD)) diff --git a/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java b/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java index 65d363b308..66bc9d61b3 100644 --- a/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java +++ b/src/benchmark/java/com/commercetools/sync/benchmark/ProductTypeSyncBenchmark.java @@ -142,7 +142,7 @@ public void sync_ExistingProductTypes_ShouldUpdateProductTypes() throws IOExcept final long totalTime = System.currentTimeMillis() - beforeSyncTime; // Calculate time taken for benchmark and assert it lies within threshold - final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_TYPE_SYNC, CREATES_ONLY, totalTime); + final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_TYPE_SYNC, UPDATES_ONLY, totalTime); assertThat(diff) .withFailMessage(format("Diff of benchmark '%e' is longer than expected threshold of '%d'.", diff, THRESHOLD)) @@ -203,7 +203,9 @@ public void sync_WithSomeExistingProductTypes_ShouldSyncProductTypes() throws IO final long totalTime = System.currentTimeMillis() - beforeSyncTime; // Calculate time taken for benchmark and assert it lies within threshold - final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_TYPE_SYNC, CREATES_ONLY, totalTime); + final double diff = + calculateDiff(SyncSolutionInfo.LIB_VERSION, PRODUCT_TYPE_SYNC, CREATES_AND_UPDATES, totalTime); + assertThat(diff) .withFailMessage(format("Diff of benchmark '%e' is longer than expected threshold of '%d'.", diff, THRESHOLD)) diff --git a/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java b/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java index 23400b64d7..425424a380 100644 --- a/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java +++ b/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java @@ -150,7 +150,7 @@ public void sync_ExistingTypes_ShouldUpdateTypes() throws IOException { final long totalTime = System.currentTimeMillis() - beforeSyncTime; // Calculate time taken for benchmark and assert it lies within threshold - final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, TYPE_SYNC, CREATES_ONLY, totalTime); + final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, TYPE_SYNC, UPDATES_ONLY, totalTime); assertThat(diff) .withFailMessage(format("Diff of benchmark '%e' is longer than expected threshold of '%d'.", diff, THRESHOLD)) @@ -211,7 +211,7 @@ public void sync_WithSomeExistingTypes_ShouldSyncTypes() throws IOException { final long totalTime = System.currentTimeMillis() - beforeSyncTime; // Calculate time taken for benchmark and assert it lies within threshold - final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, TYPE_SYNC, CREATES_ONLY, totalTime); + final double diff = calculateDiff(SyncSolutionInfo.LIB_VERSION, TYPE_SYNC, CREATES_AND_UPDATES, totalTime); assertThat(diff) .withFailMessage(format("Diff of benchmark '%e' is longer than expected threshold of '%d'.", diff, THRESHOLD)) From 1dd8d7ece45d4081e98b131c62cc28f5d8ccb94f Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:35:17 +0100 Subject: [PATCH 06/54] #300: Delete types only from target. --- .../com/commercetools/sync/benchmark/TypeSyncBenchmark.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java b/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java index 425424a380..719838d1e0 100644 --- a/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java +++ b/src/benchmark/java/com/commercetools/sync/benchmark/TypeSyncBenchmark.java @@ -38,7 +38,6 @@ import static com.commercetools.sync.benchmark.BenchmarkUtils.calculateDiff; import static com.commercetools.sync.benchmark.BenchmarkUtils.saveNewResult; import static com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics.assertThat; -import static com.commercetools.sync.integration.commons.utils.ITUtils.deleteTypesFromTargetAndSource; import static com.commercetools.sync.integration.commons.utils.SphereClientUtils.CTP_TARGET_CLIENT; import static com.commercetools.sync.integration.commons.utils.TypeITUtils.FIELD_DEFINITION_1; import static com.commercetools.sync.integration.commons.utils.TypeITUtils.FIELD_DEFINITION_NAME_1; @@ -57,7 +56,7 @@ public class TypeSyncBenchmark { @BeforeClass public static void setup() { - deleteTypesFromTargetAndSource(); + deleteTypes(CTP_TARGET_CLIENT); } @AfterClass From d29bfcd99cb691058bce4ae162c621fccbad90e6 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:38:22 +0100 Subject: [PATCH 07/54] #300: Making unneeded public fields private. --- .../commons/utils/TypeITUtils.java | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java b/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java index 294d0af38c..b057380f71 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java +++ b/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java @@ -31,13 +31,13 @@ public final class TypeITUtils { public static final LocalizedString TYPE_NAME_2 = LocalizedString.ofEnglish("name_2"); public static final String FIELD_DEFINITION_NAME_1 = "field_name_1"; - public static final String FIELD_DEFINITION_NAME_2 = "field_name_2"; - public static final String FIELD_DEFINITION_NAME_3 = "field_name_3"; + private static final String FIELD_DEFINITION_NAME_2 = "field_name_2"; + private static final String FIELD_DEFINITION_NAME_3 = "field_name_3"; public static final LocalizedString FIELD_DEFINITION_LABEL_1 = LocalizedString.ofEnglish("label_1"); - public static final LocalizedString FIELD_DEFINITION_LABEL_2 = LocalizedString.ofEnglish("label_2"); - public static final LocalizedString FIELD_DEFINITION_LABEL_3 = LocalizedString.ofEnglish("label_3"); + private static final LocalizedString FIELD_DEFINITION_LABEL_2 = LocalizedString.ofEnglish("label_2"); + private static final LocalizedString FIELD_DEFINITION_LABEL_3 = LocalizedString.ofEnglish("label_3"); public static final LocalizedString TYPE_DESCRIPTION_1 = LocalizedString.ofEnglish("description_1"); public static final LocalizedString TYPE_DESCRIPTION_2 = LocalizedString.ofEnglish("description_2"); @@ -65,23 +65,21 @@ public final class TypeITUtils { - public static final TypeDraft typeDraft1 = TypeDraftBuilder.of( - TYPE_KEY_1, - TYPE_NAME_1, - ResourceTypeIdsSetBuilder.of().addCategories().build()) - .description(TYPE_DESCRIPTION_1) - .fieldDefinitions(Arrays.asList(FIELD_DEFINITION_1, FIELD_DEFINITION_2)) - .build(); + private static final TypeDraft typeDraft1 = TypeDraftBuilder + .of(TYPE_KEY_1, + TYPE_NAME_1, + ResourceTypeIdsSetBuilder.of().addCategories().build()) + .description(TYPE_DESCRIPTION_1) + .fieldDefinitions(Arrays.asList(FIELD_DEFINITION_1, FIELD_DEFINITION_2)) + .build(); - public static final TypeDraft typeDraft2 = TypeDraftBuilder.of( - TYPE_KEY_2, + private static final TypeDraft typeDraft2 = TypeDraftBuilder + .of(TYPE_KEY_2, TYPE_NAME_2, ResourceTypeIdsSetBuilder.of().addCategories().build()) - .description(TYPE_DESCRIPTION_2) - .fieldDefinitions(singletonList(FIELD_DEFINITION_2)) - .build(); - - + .description(TYPE_DESCRIPTION_2) + .fieldDefinitions(singletonList(FIELD_DEFINITION_2)) + .build(); /** * Deletes all types from CTP project, represented by provided {@code ctpClient}. @@ -92,7 +90,6 @@ public static void deleteTypes(@Nonnull final SphereClient ctpClient) { queryAndExecute(ctpClient, TypeQuery.of(), TypeDeleteCommand::of); } - /** * Deletes all types from CTP projects defined by {@code CTP_SOURCE_CLIENT} and * {@code CTP_TARGET_CLIENT}. From c13e21a3b893f4b4f9ad3a6ed29503c395932f0b Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:39:21 +0100 Subject: [PATCH 08/54] #300: Adjust line spacing. --- .../sync/integration/commons/utils/TypeITUtils.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java b/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java index b057380f71..77f1c9b767 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java +++ b/src/integration-test/java/com/commercetools/sync/integration/commons/utils/TypeITUtils.java @@ -34,7 +34,6 @@ public final class TypeITUtils { private static final String FIELD_DEFINITION_NAME_2 = "field_name_2"; private static final String FIELD_DEFINITION_NAME_3 = "field_name_3"; - public static final LocalizedString FIELD_DEFINITION_LABEL_1 = LocalizedString.ofEnglish("label_1"); private static final LocalizedString FIELD_DEFINITION_LABEL_2 = LocalizedString.ofEnglish("label_2"); private static final LocalizedString FIELD_DEFINITION_LABEL_3 = LocalizedString.ofEnglish("label_3"); @@ -48,14 +47,12 @@ public final class TypeITUtils { FIELD_DEFINITION_LABEL_1, true, TextInputHint.SINGLE_LINE); - public static final FieldDefinition FIELD_DEFINITION_2 = FieldDefinition.of( StringFieldType.of(), FIELD_DEFINITION_NAME_2, FIELD_DEFINITION_LABEL_2, true, TextInputHint.SINGLE_LINE); - public static final FieldDefinition FIELD_DEFINITION_3 = FieldDefinition.of( StringFieldType.of(), FIELD_DEFINITION_NAME_3, @@ -63,8 +60,6 @@ public final class TypeITUtils { true, TextInputHint.SINGLE_LINE); - - private static final TypeDraft typeDraft1 = TypeDraftBuilder .of(TYPE_KEY_1, TYPE_NAME_1, @@ -72,7 +67,6 @@ public final class TypeITUtils { .description(TYPE_DESCRIPTION_1) .fieldDefinitions(Arrays.asList(FIELD_DEFINITION_1, FIELD_DEFINITION_2)) .build(); - private static final TypeDraft typeDraft2 = TypeDraftBuilder .of(TYPE_KEY_2, TYPE_NAME_2, From 903c0ca67eabefadc5d7a90598bd4c20f01863ee Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 22:45:40 +0100 Subject: [PATCH 09/54] #300: Use simpler name update. --- .../ctpprojectsource/producttypes/ProductTypeSyncIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/producttypes/ProductTypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/producttypes/ProductTypeSyncIT.java index 5df4fcadf7..650e4e7db7 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/producttypes/ProductTypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/producttypes/ProductTypeSyncIT.java @@ -108,7 +108,7 @@ public void sync_WithUpdates_ShouldReturnProperStatistics() { return ProductTypeDraftBuilder .of( productType.getKey(), - productType.getName() + "_updated", // name updated + "newName", productType.getDescription(), attributeDefinitionDrafts) .build(); From 7efd69d1508658509d36ee146d627e3210424a77 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:09:45 +0100 Subject: [PATCH 10/54] #300: Fix Javadoc. --- .../sync/integration/externalsource/types/TypeSyncIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java index de18fd858b..ed6bd91a38 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java @@ -67,8 +67,8 @@ public class TypeSyncIT { /** - * Deletes types from source and target CTP projects. - * Populates source and target CTP projects with test data. + * Deletes types from the target CTP projects. + * Populates the target CTP project with test data. */ @Before public void setup() { From 9fdb6652cf1165ec7e01ef278c62b75e0774166a Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:10:05 +0100 Subject: [PATCH 11/54] #300: Fix line spacing. --- .../sync/integration/externalsource/types/TypeSyncIT.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java index ed6bd91a38..6d4f46058a 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java @@ -731,11 +731,11 @@ public void sync__WithConcurrentModificationExceptionAndUnexpectedDelete_ShouldF assertThat(errorMessages.get(0)).contains( format("Failed to update type with key: '%s'. Reason: Not found when attempting to fetch while " + "retrying after concurrency modification.", typeDraft.getKey())); - } @Nonnull private SphereClient buildClientWithConcurrentModificationUpdateAndNotFoundFetchOnRetry() { + final SphereClient spyClient = spy(CTP_TARGET_CLIENT); final TypeQuery anyTypeQuery = any(TypeQuery.class); @@ -743,14 +743,12 @@ private SphereClient buildClientWithConcurrentModificationUpdateAndNotFoundFetch .thenCallRealMethod() // Call real fetch on fetching matching types .thenReturn(CompletableFuture.completedFuture(PagedQueryResult.empty())); - final TypeUpdateCommand anyTypeUpdate = any(TypeUpdateCommand.class); when(spyClient.execute(anyTypeUpdate)) .thenReturn(exceptionallyCompletedFuture(new ConcurrentModificationException())) .thenCallRealMethod(); - return spyClient; } From f7f8cc1d79663e45d6cb10093cfcd66666e038c1 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:10:13 +0100 Subject: [PATCH 12/54] #300: Fix Javadoc. --- .../externalsource/producttypes/ProductTypeSyncIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/producttypes/ProductTypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/producttypes/ProductTypeSyncIT.java index 2a682031be..ad26d1c08c 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/producttypes/ProductTypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/producttypes/ProductTypeSyncIT.java @@ -69,7 +69,7 @@ public class ProductTypeSyncIT { /** - * Deletes product types from target CTP project. + * Deletes product types from the target CTP project. * Populates target CTP project with test data. */ @Before From 9d66f49596a6aa30b6135e9eedc3548120274935 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:10:44 +0100 Subject: [PATCH 13/54] #300: Remove braces for 1-liner. --- .../integration/externalsource/types/TypeSyncIT.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java index 6d4f46058a..178c226df4 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java @@ -191,14 +191,9 @@ public void sync_WithUpdatedType_WithNewFieldDefinitions_ShouldUpdateTypeAddingF final Optional oldTypeAfter = getTypeByKey(CTP_TARGET_CLIENT, TYPE_KEY_1); - assertThat(oldTypeAfter).hasValueSatisfying(type -> { + assertThat(oldTypeAfter).hasValueSatisfying(type -> assertFieldDefinitionsAreEqual(type.getFieldDefinitions(), - asList( - FIELD_DEFINITION_1, - FIELD_DEFINITION_2, - FIELD_DEFINITION_3 - )); - }); + asList(FIELD_DEFINITION_1, FIELD_DEFINITION_2, FIELD_DEFINITION_3))); } @Test From 1154d69002e9ccbc099e76c4047d549c116955a8 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:10:59 +0100 Subject: [PATCH 14/54] #300: Use optional assertion. --- .../sync/integration/externalsource/types/TypeSyncIT.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java index 178c226df4..2edc8a52f3 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java @@ -223,9 +223,8 @@ public void sync_WithUpdatedType_WithoutOldFieldDefinition_ShouldUpdateTypeRemov final Optional oldTypeAfter = getTypeByKey(CTP_TARGET_CLIENT, TYPE_KEY_1); - assertThat(oldTypeAfter).isNotEmpty(); - assertFieldDefinitionsAreEqual(oldTypeAfter.get().getFieldDefinitions(), - singletonList(FIELD_DEFINITION_1)); + assertThat(oldTypeAfter).hasValueSatisfying(type -> + assertFieldDefinitionsAreEqual(type.getFieldDefinitions(), singletonList(FIELD_DEFINITION_1))); } @Test From 32d00af95638292cb81fca9517a0056c0f4e6367 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:11:31 +0100 Subject: [PATCH 15/54] #300: Remove unneeded variable reassignment. --- .../sync/integration/externalsource/types/TypeSyncIT.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java index 2edc8a52f3..e7d23fb7a5 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java @@ -574,15 +574,14 @@ public void sync_WithConcurrentModificationException_ShouldRetryToUpdateNewTypeW final SphereClient spyClient = buildClientWithConcurrentModificationUpdate(); final TypeDraft typeDraft = TypeDraftBuilder - .of("typeKey", LocalizedString.ofEnglish( "typeName"), ResourceTypeIdsSetBuilder.of().addChannels()) + .of(TYPE_KEY_2, TYPE_NAME_2, ResourceTypeIdsSetBuilder.of().addChannels()) .build(); CTP_TARGET_CLIENT.execute(TypeCreateCommand.of(typeDraft)) .toCompletableFuture() .join(); - final LocalizedString newTypeName = TYPE_NAME_2; - final TypeDraft updatedDraft = TypeDraftBuilder.of(typeDraft).name(newTypeName).build(); + final TypeDraft updatedDraft = TypeDraftBuilder.of(typeDraft).name(TYPE_NAME_1).build(); final TypeSyncOptions typeSyncOptions = TypeSyncOptionsBuilder.of(spyClient).build(); final TypeSync typeSync = new TypeSync(typeSyncOptions); @@ -603,7 +602,7 @@ public void sync_WithConcurrentModificationException_ShouldRetryToUpdateNewTypeW .join(); assertThat(queryResult.head()).hasValueSatisfying(type -> - assertThat(type.getName()).isEqualTo(newTypeName)); + assertThat(type.getName()).isEqualTo(TYPE_NAME_1)); } @Nonnull From 7fe0b60fd37c24295d4019f0573977c19254e50c Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:47:16 +0100 Subject: [PATCH 16/54] #300: Move method and remove key check since it's required by CTP. --- .../sync/services/impl/TypeServiceImpl.java | 31 ++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java b/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java index 978e373398..21c16f678b 100644 --- a/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java +++ b/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java @@ -22,9 +22,7 @@ import java.util.function.Consumer; import java.util.function.Function; -import static java.lang.String.format; import static java.util.stream.Collectors.toSet; -import static org.apache.commons.lang3.StringUtils.isNotBlank; import static org.apache.http.util.TextUtils.isBlank; /** @@ -48,6 +46,16 @@ public CompletionStage> fetchCachedTypeId(@Nonnull final String return fetchAndCache(key); } + @Nonnull + private CompletionStage> fetchAndCache(@Nonnull final String key) { + + final Consumer> typePageConsumer = typePage -> + typePage.forEach(type -> keyToIdCache.put(type.getKey(), type.getId())); + + return CtpQueryUtils.queryAll(syncOptions.getCtpClient(), TypeQuery.of(), typePageConsumer) + .thenApply(result -> Optional.ofNullable(keyToIdCache.get(key))); + } + @Nonnull @Override public CompletionStage> fetchMatchingTypesByKeys(@Nonnull final Set keys) { @@ -104,23 +112,4 @@ public CompletionStage updateType(@Nonnull final Type type, return updateResource(type, TypeUpdateCommand::of, updateActions); } - - @Nonnull - private CompletionStage> fetchAndCache(@Nonnull final String key) { - - final Consumer> typePageConsumer = typePage -> - typePage.forEach(type -> { - final String fetchedTypeKey = type.getKey(); - final String id = type.getId(); - if (isNotBlank(fetchedTypeKey)) { - keyToIdCache.put(fetchedTypeKey, id); - } else { - syncOptions.applyWarningCallback(format("Type with id: '%s' has no key set. Keys are" - + " required for type matching.", id)); - } - }); - - return CtpQueryUtils.queryAll(syncOptions.getCtpClient(), TypeQuery.of(), typePageConsumer) - .thenApply(result -> Optional.ofNullable(keyToIdCache.get(key))); - } } From 28ed07adccde222d963eb719306e3523759b62da Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:47:34 +0100 Subject: [PATCH 17/54] #300: Remove unneeded variable and format method sig. --- .../commercetools/sync/services/impl/TypeServiceImpl.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java b/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java index 21c16f678b..77d96a7901 100644 --- a/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java +++ b/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java @@ -30,7 +30,6 @@ * TODO: USE graphQL to get only keys. GITHUB ISSUE#84 */ public final class TypeServiceImpl extends BaseService implements TypeService { - private static final String FETCH_FAILED = "Failed to fetch types with keys: '%s'. Reason: %s"; public TypeServiceImpl(@Nonnull final BaseSyncOptions syncOptions) { super(syncOptions); @@ -107,9 +106,9 @@ public CompletionStage> createType(@Nonnull final TypeDraft typeD @Nonnull @Override - public CompletionStage updateType(@Nonnull final Type type, - @Nonnull final List> updateActions) { - + public CompletionStage updateType( + @Nonnull final Type type, + @Nonnull final List> updateActions) { return updateResource(type, TypeUpdateCommand::of, updateActions); } } From 94e90198eef1f900decbb8862fa3a8b1e1853110 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:47:53 +0100 Subject: [PATCH 18/54] #300: Fix method names. --- .../sync/services/impl/ProductTypeServiceImplTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/commercetools/sync/services/impl/ProductTypeServiceImplTest.java b/src/test/java/com/commercetools/sync/services/impl/ProductTypeServiceImplTest.java index b727aa35ed..8b007be167 100644 --- a/src/test/java/com/commercetools/sync/services/impl/ProductTypeServiceImplTest.java +++ b/src/test/java/com/commercetools/sync/services/impl/ProductTypeServiceImplTest.java @@ -23,7 +23,7 @@ public class ProductTypeServiceImplTest { @Test - public void fetchProductType_WithEmptyKey_ShouldNotFetchCategory() { + public void fetchProductType_WithEmptyKey_ShouldNotFetchProductType() { // preparation final SphereClient sphereClient = mock(SphereClient.class); final ProductTypeSyncOptions syncOptions = ProductTypeSyncOptionsBuilder @@ -40,7 +40,7 @@ public void fetchProductType_WithEmptyKey_ShouldNotFetchCategory() { } @Test - public void fetchProductType_WithNullKey_ShouldNotFetchCategory() { + public void fetchProductType_WithNullKey_ShouldNotFetchProductType() { // preparation final SphereClient sphereClient = mock(SphereClient.class); final ProductTypeSyncOptions syncOptions = ProductTypeSyncOptionsBuilder From f5895cdc7d92be74b13d510a81135f0872e71e0b Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:48:34 +0100 Subject: [PATCH 19/54] #300: Statically import, improve indentation and remove unneeded assertion. --- .../services/impl/TypeServiceImplIT.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java b/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java index e2572c9f7f..94f6d6bb53 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java @@ -8,7 +8,6 @@ import io.sphere.sdk.client.BadGatewayException; import io.sphere.sdk.client.ErrorResponseException; import io.sphere.sdk.client.SphereClient; -import io.sphere.sdk.models.LocalizedString; import io.sphere.sdk.models.errors.DuplicateFieldError; import io.sphere.sdk.types.ResourceTypeIdsSetBuilder; import io.sphere.sdk.types.Type; @@ -39,6 +38,7 @@ import static com.commercetools.sync.integration.commons.utils.TypeITUtils.TYPE_NAME_1; import static com.commercetools.sync.integration.commons.utils.TypeITUtils.deleteTypes; import static com.commercetools.tests.utils.CompletionStageUtil.executeBlocking; +import static io.sphere.sdk.models.LocalizedString.ofEnglish; import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static java.util.stream.Collectors.toList; @@ -69,12 +69,13 @@ public void setup() { deleteTypes(CTP_TARGET_CLIENT); createCategoriesCustomType(OLD_TYPE_KEY, OLD_TYPE_LOCALE, OLD_TYPE_NAME, CTP_TARGET_CLIENT); - final TypeSyncOptions typeSyncOptions = TypeSyncOptionsBuilder.of(CTP_TARGET_CLIENT) - .errorCallback((errorMessage, exception) -> { - errorCallBackMessages.add(errorMessage); - errorCallBackExceptions.add(exception); - }) - .build(); + final TypeSyncOptions typeSyncOptions = TypeSyncOptionsBuilder + .of(CTP_TARGET_CLIENT) + .errorCallback((errorMessage, exception) -> { + errorCallBackMessages.add(errorMessage); + errorCallBackExceptions.add(exception); + }) + .build(); typeService = new TypeServiceImpl(typeSyncOptions); } @@ -142,7 +143,6 @@ public void fetchMatchingTypesByKeys_WithAnyExistingKeys_ShouldReturnASetOfTypes .toCompletableFuture() .join(); - assertThat(matchingTypes).isNotEmpty(); assertThat(matchingTypes).hasSize(1); assertThat(errorCallBackExceptions).isEmpty(); assertThat(errorCallBackMessages).isEmpty(); @@ -344,7 +344,7 @@ public void updateType_WithValidChanges_ShouldUpdateTypeCorrectly() { .toCompletableFuture().join().head(); assertThat(typeOptional).isNotNull(); - final ChangeName changeNameUpdateAction = ChangeName.of(LocalizedString.ofEnglish("new_type_name")); + final ChangeName changeNameUpdateAction = ChangeName.of(ofEnglish("new_type_name")); final Type updatedType = typeService.updateType(typeOptional.get(), singletonList(changeNameUpdateAction)) .toCompletableFuture().join(); From 98c7b509c398b826c2a4fb524b0ea86dc5e38a12 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Wed, 5 Dec 2018 23:50:09 +0100 Subject: [PATCH 20/54] #300: Fix typos. --- .../sync/integration/services/impl/TypeServiceImplIT.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java b/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java index 94f6d6bb53..f129354267 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/services/impl/TypeServiceImplIT.java @@ -180,9 +180,9 @@ public void fetchMatchingTypesByKeys_WithBadGateWayExceptionAlways_ShouldFail() @Test public void fetchMatchingTypesByKeys_WithAllExistingSetOfKeys_ShouldCacheFetchedTypeIds() { - final Set fetchedProductTypes = typeService.fetchMatchingTypesByKeys(singleton(OLD_TYPE_KEY)) - .toCompletableFuture().join(); - assertThat(fetchedProductTypes).hasSize(1); + final Set fetchedTypes = typeService.fetchMatchingTypesByKeys(singleton(OLD_TYPE_KEY)) + .toCompletableFuture().join(); + assertThat(fetchedTypes).hasSize(1); final Optional typeOptional = CTP_TARGET_CLIENT .execute(TypeQuery.of().withPredicates(queryModel -> queryModel.key().is(OLD_TYPE_KEY))) @@ -288,7 +288,7 @@ public void createType_WithInvalidType_ShouldHaveEmptyOptionalAsAResult() { } @Test - public void createProductType_WithDuplicateKey_ShouldHaveEmptyOptionalAsAResult() { + public void createType_WithDuplicateKey_ShouldHaveEmptyOptionalAsAResult() { //preparation final TypeDraft newTypeDraft = TypeDraftBuilder.of( OLD_TYPE_KEY, From f5da17e837d85e34b1fabdde4d6232361141daf7 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:11:44 +0100 Subject: [PATCH 21/54] #300: Remove redundant supressions. --- .../com/commercetools/sync/producttypes/ProductTypeSync.java | 1 - src/main/java/com/commercetools/sync/types/TypeSync.java | 1 - 2 files changed, 2 deletions(-) diff --git a/src/main/java/com/commercetools/sync/producttypes/ProductTypeSync.java b/src/main/java/com/commercetools/sync/producttypes/ProductTypeSync.java index 608124d674..549ded85aa 100644 --- a/src/main/java/com/commercetools/sync/producttypes/ProductTypeSync.java +++ b/src/main/java/com/commercetools/sync/producttypes/ProductTypeSync.java @@ -208,7 +208,6 @@ private CompletionStage syncBatch( * @param productTypeDraft the product type draft to create the product type from. * @return a {@link CompletionStage} which contains an empty result after execution of the create. */ - @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") // https://github.com/findbugsproject/findbugs/issues/79 @Nonnull private CompletionStage> applyCallbackAndCreate( @Nonnull final ProductTypeDraft productTypeDraft) { diff --git a/src/main/java/com/commercetools/sync/types/TypeSync.java b/src/main/java/com/commercetools/sync/types/TypeSync.java index 34a2cf2be6..25f9153c3c 100644 --- a/src/main/java/com/commercetools/sync/types/TypeSync.java +++ b/src/main/java/com/commercetools/sync/types/TypeSync.java @@ -199,7 +199,6 @@ private CompletionStage syncBatch( * @param typeDraft the type draft to create the type from. * @return a {@link CompletionStage} which contains an empty result after execution of the create. */ - @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") // https://github.com/findbugsproject/findbugs/issues/79 @Nonnull private CompletionStage> applyCallbackAndCreate( @Nonnull final TypeDraft typeDraft) { From 7d7706788c5da862d89309e8880967adf4cbd37d Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:12:06 +0100 Subject: [PATCH 22/54] #300: Add @nonnull annotation. --- src/main/java/com/commercetools/sync/types/TypeSync.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/commercetools/sync/types/TypeSync.java b/src/main/java/com/commercetools/sync/types/TypeSync.java index 25f9153c3c..7bb0edd11b 100644 --- a/src/main/java/com/commercetools/sync/types/TypeSync.java +++ b/src/main/java/com/commercetools/sync/types/TypeSync.java @@ -282,6 +282,7 @@ private CompletionStage> updateType( }); } + @Nonnull private CompletionStage> fetchAndUpdate( @Nonnull final Type oldType, @Nonnull final TypeDraft newType) { From 694042f1f2e0fece9854def4e014be50b79c4be2 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:12:18 +0100 Subject: [PATCH 23/54] #300: Statically import util. --- src/main/java/com/commercetools/sync/types/TypeSync.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/TypeSync.java b/src/main/java/com/commercetools/sync/types/TypeSync.java index 7bb0edd11b..b4af838414 100644 --- a/src/main/java/com/commercetools/sync/types/TypeSync.java +++ b/src/main/java/com/commercetools/sync/types/TypeSync.java @@ -4,7 +4,6 @@ import com.commercetools.sync.services.TypeService; import com.commercetools.sync.services.impl.TypeServiceImpl; import com.commercetools.sync.types.helpers.TypeSyncStatistics; -import com.commercetools.sync.types.utils.TypeSyncUtils; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.types.Type; @@ -21,6 +20,7 @@ import java.util.concurrent.CompletionStage; import static com.commercetools.sync.commons.utils.SyncUtils.batchElements; +import static com.commercetools.sync.types.utils.TypeSyncUtils.buildActions; import static java.lang.String.format; import static java.util.Optional.ofNullable; import static java.util.concurrent.CompletableFuture.completedFuture; @@ -225,7 +225,7 @@ private CompletionStage> buildActionsAndUpdate( @Nonnull final Type oldType, @Nonnull final TypeDraft newType) { - final List> updateActions = TypeSyncUtils.buildActions(oldType, newType, syncOptions); + final List> updateActions = buildActions(oldType, newType, syncOptions); final List> updateActionsAfterCallback = syncOptions.applyBeforeUpdateCallBack(updateActions, newType, oldType); From 9057af91bc0f53b3d0fd494ec77442658a7e012d Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:15:24 +0100 Subject: [PATCH 24/54] #300: Remove excessive line spacing. --- src/test/java/com/commercetools/sync/types/TypeSyncTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/TypeSyncTest.java b/src/test/java/com/commercetools/sync/types/TypeSyncTest.java index 40c0fb36ed..455da3671c 100644 --- a/src/test/java/com/commercetools/sync/types/TypeSyncTest.java +++ b/src/test/java/com/commercetools/sync/types/TypeSyncTest.java @@ -122,8 +122,6 @@ public void sync_WithOnlyDraftsToUpdate_ShouldOnlyCallBeforeUpdateCallback() { .of(mock(SphereClient.class)) .build(); - - final Type mockedExistingType = mock(Type.class); when(mockedExistingType.getKey()).thenReturn(newTypeDraft.getKey()); From a08da1f01d14ec734ad67ff2bad9bdd1728e2b0c Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:25:48 +0100 Subject: [PATCH 25/54] #300: Refactor TypeSyncUtils. --- .../sync/types/utils/TypeSyncUtils.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java b/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java index 2d67248522..272c663ebd 100644 --- a/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java @@ -7,13 +7,11 @@ import javax.annotation.Nonnull; import java.util.List; -import java.util.Optional; -import java.util.stream.Stream; +import static com.commercetools.sync.commons.utils.OptionalUtils.filterEmptyOptionals; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildChangeNameUpdateAction; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildFieldDefinitionUpdateActions; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildSetDescriptionUpdateAction; -import static java.util.stream.Collectors.toList; public final class TypeSyncUtils { @@ -26,7 +24,12 @@ public final class TypeSyncUtils { * {@link TypeDraft} have the same fields, an empty {@link List} is returned. * *

- * TODO: Check GITHUB ISSUE#339 for missing FieldDefinition update actions. + * Note: Currently this util doesn't support the following: + *

    + *
  • updating the inputHint of a FieldDefinition
    • + *
  • removing the EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
  • updating the label of a EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
*

* * @param oldType the {@link Type} which should be updated. @@ -43,13 +46,11 @@ public static List> buildActions( @Nonnull final TypeDraft newType, @Nonnull final TypeSyncOptions syncOptions) { - final List> updateActions = Stream.of( + final List> updateActions = + filterEmptyOptionals( buildChangeNameUpdateAction(oldType, newType), buildSetDescriptionUpdateAction(oldType, newType) - ) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(toList()); + ); updateActions.addAll(buildFieldDefinitionUpdateActions(oldType, newType, syncOptions)); From f9d086f8dc9ca9bd5ec410849c84c1374030b55d Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:31:41 +0100 Subject: [PATCH 26/54] #300: Refactor util name. --- .../sync/types/utils/TypeSyncUtils.java | 6 ++-- .../types/utils/TypeUpdateActionUtils.java | 7 ++-- ...BuildFieldDefinitionUpdateActionsTest.java | 32 +++++++++---------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java b/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java index 272c663ebd..e5a9361a85 100644 --- a/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java @@ -10,14 +10,14 @@ import static com.commercetools.sync.commons.utils.OptionalUtils.filterEmptyOptionals; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildChangeNameUpdateAction; -import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildFieldDefinitionUpdateActions; +import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildFieldDefinitionsUpdateActions; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildSetDescriptionUpdateAction; public final class TypeSyncUtils { /** * Compares all the fields (including the field definitions see - * {@link TypeUpdateActionUtils#buildFieldDefinitionUpdateActions(Type, TypeDraft, TypeSyncOptions)}) + * {@link TypeUpdateActionUtils#buildFieldDefinitionsUpdateActions(Type, TypeDraft, TypeSyncOptions)}) * of a {@link Type} and a {@link TypeDraft}. * It returns a {@link List} of {@link UpdateAction}<{@link Type}> as a * result. If no update actions are needed, for example in case where both the {@link Type} and the @@ -52,7 +52,7 @@ public static List> buildActions( buildSetDescriptionUpdateAction(oldType, newType) ); - updateActions.addAll(buildFieldDefinitionUpdateActions(oldType, newType, syncOptions)); + updateActions.addAll(buildFieldDefinitionsUpdateActions(oldType, newType, syncOptions)); return updateActions; } diff --git a/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java index 05750410b6..7897420c7d 100644 --- a/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java @@ -15,7 +15,6 @@ import java.util.Optional; import static com.commercetools.sync.commons.utils.CommonTypeUpdateActionUtils.buildUpdateAction; -import static com.commercetools.sync.types.utils.FieldDefinitionsUpdateActionUtils.buildFieldDefinitionsUpdateActions; import static java.lang.String.format; import static java.util.Collections.emptyList; @@ -53,6 +52,7 @@ public static Optional> buildChangeNameUpdateAction( public static Optional> buildSetDescriptionUpdateAction( @Nonnull final Type oldType, @Nonnull final TypeDraft newType) { + return buildUpdateAction(oldType.getDescription(), newType.getDescription(), () -> SetDescription.of(newType.getDescription())); } @@ -75,12 +75,13 @@ public static Optional> buildSetDescriptionUpdateAction( * @return A list with the update actions or an empty list if the field definitions are identical. */ @Nonnull - public static List> buildFieldDefinitionUpdateActions( + public static List> buildFieldDefinitionsUpdateActions( @Nonnull final Type oldType, @Nonnull final TypeDraft newType, @Nonnull final TypeSyncOptions syncOptions) { + try { - return buildFieldDefinitionsUpdateActions( + return FieldDefinitionsUpdateActionUtils.buildFieldDefinitionsUpdateActions( oldType.getFieldDefinitions(), newType.getFieldDefinitions() ); diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java index c60b5b9c36..26a7622710 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java @@ -25,7 +25,7 @@ import static com.commercetools.sync.types.FieldDefinitionTestHelper.localizedStringFieldDefinition; import static com.commercetools.sync.types.FieldDefinitionTestHelper.stringFieldDefinition; -import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildFieldDefinitionUpdateActions; +import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildFieldDefinitionsUpdateActions; import static io.sphere.sdk.json.SphereJsonUtils.readObjectFromResource; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; @@ -98,7 +98,7 @@ public class BuildFieldDefinitionUpdateActionsTest { public void buildUpdateActions_WithNullNewFieldDefinitionsAndExistingFieldDefinitions_ShouldBuild3RemoveActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, TYPE_DRAFT, SYNC_OPTIONS @@ -117,7 +117,7 @@ public void buildUpdateActions_WithNullNewFieldDefinitionsAndNoOldFieldDefinitio when(oldType.getFieldDefinitions()).thenReturn(emptyList()); when(oldType.getKey()).thenReturn("type_key_1"); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, TYPE_DRAFT, SYNC_OPTIONS @@ -137,7 +137,7 @@ public void buildUpdateActions_WithNewFieldDefinitionsAndNoOldFieldDefinitions_S TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -159,7 +159,7 @@ public void buildUpdateActions_WithIdenticalFieldDefinitions_ShouldNotBuildUpdat TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -186,7 +186,7 @@ public void buildUpdateActions_WithDuplicateFieldDefinitionNames_ShouldNotBuildA }) .build(); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, syncOptions @@ -215,7 +215,7 @@ public void buildUpdateActions_WithOneMissingFieldDefinition_ShouldBuildRemoveFi TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -233,7 +233,7 @@ public void buildUpdateActions_WithOneExtraFieldDefinition_ShouldBuildAddFieldDe TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -253,7 +253,7 @@ public void buildUpdateActions_WithOneFieldDefinitionSwitch_ShouldBuildRemoveAnd TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -274,7 +274,7 @@ public void buildUpdateActions_WithDifferent_ShouldBuildChangeFieldDefinitionOrd TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -300,7 +300,7 @@ public void buildUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrd TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -326,7 +326,7 @@ public void buildUpdateActions_WithAddedAndDifferentOrder_ShouldBuildChangeOrder TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -354,7 +354,7 @@ public void buildUpdateActions_WithAddedFieldDefinitionInBetween_ShouldBuildChan TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -382,7 +382,7 @@ public void buildUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAllThreeMo TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -408,7 +408,7 @@ public void buildUpdateActions_WithDifferentType_ShouldRemoveOldFieldDefinitionA TYPE_WITH_FIELDS_ABC_WITH_DIFFERENT_TYPE, TypeDraft.class ); - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, newTypeDraft, SYNC_OPTIONS @@ -447,7 +447,7 @@ public void buildUpdateActions_WithNullNewFieldDefinition_ShouldSkipNullFieldDef .build(); // test - final List> updateActions = buildFieldDefinitionUpdateActions( + final List> updateActions = buildFieldDefinitionsUpdateActions( oldType, typeDraft, SYNC_OPTIONS From cfc15828ed3c79990d2c4bcda8e03d61cd1c5c98 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 09:31:59 +0100 Subject: [PATCH 27/54] #300: Fix Javadoc. --- .../sync/types/utils/TypeUpdateActionUtils.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java index 7897420c7d..ad64083f75 100644 --- a/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java @@ -59,11 +59,17 @@ public static Optional> buildSetDescriptionUpdateAction( /** * Compares the field definitions of a {@link Type} and a {@link TypeDraft} and returns a list of - * {@link UpdateAction}<{@link Type}> as a result in an {@link Optional} of update action - * if values are different. In case, the new type draft has a list of field definitions in which a - * duplicate name exists, the error callback is triggered and an empty list is returned. + * {@link UpdateAction}<{@link Type}> as a result if the values are different. In case, the new type draft has + * a list of field definitions in which a duplicate name exists, the error callback is triggered and an empty list + * is returned. * *

+ * Note: Currently this util doesn't support the following: + *

    + *
  • updating the inputHint of a FieldDefinition
    • + *
  • removing the EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
  • updating the label of a EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
* TODO: Check GITHUB ISSUE#339 for missing FieldDefinition update actions. *

* From 577a8500fca1336e5360503c6a5ec42c9646959f Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:16:47 +0100 Subject: [PATCH 28/54] #300: Fix indentation. --- .../FieldDefinitionsUpdateActionUtils.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java index c1306f28a9..b8caf4e249 100644 --- a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java @@ -135,17 +135,14 @@ private static List> buildRemoveFieldDefinitionOrFieldDefinit @Nonnull final List newFieldDefinitions) { final Map newFieldDefinitionsNameMap = - newFieldDefinitions - .stream().collect( - toMap(FieldDefinition::getName, fieldDefinition -> fieldDefinition, - (fieldDefinitionA, fieldDefinitionB) -> { - throw new DuplicateNameException(format("Field definitions have duplicated names. " - + "Duplicated field definition name: '%s'. " - + "Field definitions names are expected " - + "to be unique inside their type.", - fieldDefinitionA.getName())); - } - )); + newFieldDefinitions + .stream().collect( + toMap(FieldDefinition::getName, fieldDefinition -> fieldDefinition, + (fieldDefinitionA, fieldDefinitionB) -> { + throw new DuplicateNameException(format("Field definitions have duplicated names. " + + "Duplicated field definition name: '%s'. Field definitions names are " + + "expected to be unique inside their type.", fieldDefinitionA.getName())); + })); return oldFieldDefinitions .stream() From fd5cff6c826875f913ed4ceb1cf10d991afad893 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:17:14 +0100 Subject: [PATCH 29/54] #300: Fix Javadoc. --- .../sync/types/utils/FieldDefinitionsUpdateActionUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java index b8caf4e249..8c8bb259a9 100644 --- a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java @@ -118,7 +118,7 @@ private static List> buildUpdateActions( * Otherwise, if the field definition still exists in the new field definition, then compare the field definition * fields (label, etc..), and add the computed actions to the list of update actions. * - *

Note: If the field type field changes, the old field definition is removed and the new field + *

Note: If the field type field is different, the old field definition is removed and the new field * definition is added with the new field type. * * @param oldFieldDefinitions the list of old {@link FieldDefinition}s. @@ -238,7 +238,7 @@ private static Optional> buildChangeFieldDefinitionOrderUpdat /** * Checks if there are any new field definition drafts which are not existing in the - * {@code oldFieldDefinitionNameMap}. If there are, then "add" field definition update actions are built. + * {@code oldFieldDefinitions}. If there are, then "add" field definition update actions are built. * Otherwise, if there are no new field definitions, then an empty list is returned. * * @param oldFieldDefinitions the list of old {@link FieldDefinition}s. From 85dab403ca49418722cea3420d3c7956ea0faad8 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:17:35 +0100 Subject: [PATCH 30/54] #300: Add note about FieldType and AttributeType changes. --- docs/usage/PRODUCT_TYPE_SYNC.md | 8 +++++++- docs/usage/TYPE_SYNC.md | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/usage/PRODUCT_TYPE_SYNC.md b/docs/usage/PRODUCT_TYPE_SYNC.md index 3bf1604ab7..3f0c69bdda 100644 --- a/docs/usage/PRODUCT_TYPE_SYNC.md +++ b/docs/usage/PRODUCT_TYPE_SYNC.md @@ -12,6 +12,8 @@ against a [ProductTypeDraft](https://docs.commercetools.com/http-api-projects-pr - [Sync list of product type drafts](#sync-list-of-product-type-drafts) - [Prerequisites](#prerequisites) - [Running the sync](#running-the-sync) + - [Important to Note](#important-to-note) + - [More examples of how to use the sync](#more-examples-of-how-to-use-the-sync) - [Build all update actions](#build-all-update-actions) - [Build particular update action(s)](#build-particular-update-actions) - [Caveats](#caveats) @@ -64,8 +66,12 @@ __Note__ The statistics object contains the processing time of the last batch on 1. The sync processing time should not take into account the time between supplying batches to the sync. 2. It is not known by the sync which batch is going to be the last one supplied. + +#### Important to Note +1. If two matching `attributeDefinition`s on the matching `productType`s have a different `AttributeType`, the sync will +**remove** the existing `attributeDefinition` and then **add** a new `attributeDefinition` with the new `AttributeType`. -##### More examples of how to use the sync +#### More examples of how to use the sync 1. [Sync from another CTP project as a source](https://github.com/commercetools/commercetools-sync-java/tree/master/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/producttypes/ProductTypeSyncIT.java). 2. [Sync from an external source](https://github.com/commercetools/commercetools-sync-java/tree/master/src/integration-test/java/com/commercetools/sync/integration/externalsource/producttypes/ProductTypeSyncIT.java). diff --git a/docs/usage/TYPE_SYNC.md b/docs/usage/TYPE_SYNC.md index a4bafb484d..6574e8881e 100644 --- a/docs/usage/TYPE_SYNC.md +++ b/docs/usage/TYPE_SYNC.md @@ -12,6 +12,8 @@ against a [TypeDraft](https://docs.commercetools.com/http-api-projects-types.htm - [Sync list of type drafts](#sync-list-of-type-drafts) - [Prerequisites](#prerequisites) - [Running the sync](#running-the-sync) + - [Important to Note](#important-to-note) + - [More examples of how to use the sync](#more-examples-of-how-to-use-the-sync) - [Build all update actions](#build-all-update-actions) - [Build particular update action(s)](#build-particular-update-actions) - [Caveats](#caveats) @@ -62,7 +64,11 @@ __Note__ The statistics object contains the processing time of the last batch on 1. The sync processing time should not take into account the time between supplying batches to the sync. 2. It is not known by the sync which batch is going to be the last one supplied. -##### More examples of how to use the sync +#### Important to Note +1. If two matching `fieldDefinition`s on the matching `type`s have a different `FieldType`, the sync will +**remove** the existing `fieldDefinition` and then **add** a new `fieldDefinition` with the new `FieldType`. + +#### More examples of how to use the sync 1. [Sync from another CTP project as a source](https://github.com/commercetools/commercetools-sync-java/tree/master/src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/types/TypeSyncIT.java). 2. [Sync from an external source](https://github.com/commercetools/commercetools-sync-java/tree/master/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java). From bd01fdb63e3ecb317bd42a69926ae82b65a185cd Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:17:55 +0100 Subject: [PATCH 31/54] #300: Add a test case for a new draft with different order. --- .../sync/integration/externalsource/types/TypeSyncIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java index e7d23fb7a5..dcba42a4e5 100644 --- a/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java +++ b/src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java @@ -235,7 +235,7 @@ public void sync_WithUpdatedType_ChangingFieldDefinitionOrder_ShouldUpdateTypeCh TYPE_NAME_1, ResourceTypeIdsSetBuilder.of().addCategories().build()) .description(TYPE_DESCRIPTION_1) - .fieldDefinitions(asList(FIELD_DEFINITION_2, FIELD_DEFINITION_1)) + .fieldDefinitions(asList(FIELD_DEFINITION_2, FIELD_DEFINITION_3, FIELD_DEFINITION_1)) .build(); final TypeSyncOptions typeSyncOptions = TypeSyncOptionsBuilder @@ -256,7 +256,7 @@ public void sync_WithUpdatedType_ChangingFieldDefinitionOrder_ShouldUpdateTypeCh assertThat(oldTypeAfter).hasValueSatisfying(type -> assertFieldDefinitionsAreEqual(type.getFieldDefinitions(), - asList(FIELD_DEFINITION_2, FIELD_DEFINITION_1))); + asList(FIELD_DEFINITION_2, FIELD_DEFINITION_3, FIELD_DEFINITION_1))); } @Test From 2128a66ca65f4d568c28a24b44158aad5f567073 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:18:10 +0100 Subject: [PATCH 32/54] #300: Remove unneeded asserts. --- .../sync/types/utils/TypeUpdateActionUtilsTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java b/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java index 2b1f9243fd..63a24d6bb6 100644 --- a/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java @@ -74,7 +74,6 @@ public static void setup() { public void buildChangeNameAction_WithDifferentValues_ShouldReturnAction() { final Optional> result = buildChangeNameUpdateAction(old, newDifferent); - assertThat(result).containsInstanceOf(ChangeName.class); assertThat(result).contains(ChangeName.of(newDifferent.getName())); } @@ -89,7 +88,6 @@ public void buildChangeNameAction_WithSameValues_ShouldReturnEmptyOptional() { public void buildSetDescriptionAction_WithDifferentValues_ShouldReturnAction() { final Optional> result = buildSetDescriptionUpdateAction(old, newDifferent); - assertThat(result).containsInstanceOf(SetDescription.class); assertThat(result).contains(SetDescription.of(newDifferent.getDescription())); } From 658a005906b0bd55846c64e802b0af917d8d99a1 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:18:45 +0100 Subject: [PATCH 33/54] #300: Add a unit test with a definition with a null name. --- ...BuildFieldDefinitionUpdateActionsTest.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java index 26a7622710..68baa51b2f 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java @@ -457,4 +457,43 @@ public void buildUpdateActions_WithNullNewFieldDefinition_ShouldSkipNullFieldDef assertThat(updateActions).containsExactly( ChangeFieldDefinitionLabel.of(newFieldDefinition.getName(), newFieldDefinition.getLabel())); } + + @Test + public void buildUpdateActions_WithDefinitionWithNullName_ShouldBuildChangeFieldDefinitionOrderAction() { + // preparation + final Type oldType = mock(Type.class); + final FieldDefinition oldFieldDefinition = FieldDefinition.of( + LocalizedEnumFieldType.of(emptyList()), + "field_1", + LocalizedString.ofEnglish("label1"), + false, + TextInputHint.SINGLE_LINE); + + + when(oldType.getFieldDefinitions()).thenReturn(singletonList(oldFieldDefinition)); + + final FieldDefinition newFieldDefinition = FieldDefinition.of( + LocalizedEnumFieldType.of(emptyList()), + null, + LocalizedString.ofEnglish("label2"), + false, + TextInputHint.SINGLE_LINE); + + final TypeDraft typeDraft = TypeDraftBuilder + .of("key", LocalizedString.ofEnglish("label"), emptySet()) + .fieldDefinitions(asList(null, newFieldDefinition)) + .build(); + + // test + final List> updateActions = buildFieldDefinitionsUpdateActions( + oldType, + typeDraft, + SYNC_OPTIONS + ); + + // assertion + assertThat(updateActions).containsExactly( + RemoveFieldDefinition.of(oldFieldDefinition.getName()), + AddFieldDefinition.of(newFieldDefinition)); + } } From d3f8dd8af23c4137ae9da5d817bfc6a710a525d7 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:21:25 +0100 Subject: [PATCH 34/54] #300: Fix unit test names. --- ...BuildFieldDefinitionUpdateActionsTest.java | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java index 68baa51b2f..2d54000da7 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java @@ -95,7 +95,7 @@ public class BuildFieldDefinitionUpdateActionsTest { .build(); @Test - public void buildUpdateActions_WithNullNewFieldDefinitionsAndExistingFieldDefinitions_ShouldBuild3RemoveActions() { + public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndExistingFieldDefinitions_ShouldBuild3RemoveActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final List> updateActions = buildFieldDefinitionsUpdateActions( @@ -112,7 +112,7 @@ public void buildUpdateActions_WithNullNewFieldDefinitionsAndExistingFieldDefini } @Test - public void buildUpdateActions_WithNullNewFieldDefinitionsAndNoOldFieldDefinitions_ShouldNotBuildActions() { + public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndNoOldFieldDefinitions_ShouldNotBuildActions() { final Type oldType = mock(Type.class); when(oldType.getFieldDefinitions()).thenReturn(emptyList()); when(oldType.getKey()).thenReturn("type_key_1"); @@ -127,7 +127,7 @@ public void buildUpdateActions_WithNullNewFieldDefinitionsAndNoOldFieldDefinitio } @Test - public void buildUpdateActions_WithNewFieldDefinitionsAndNoOldFieldDefinitions_ShouldBuild3AddActions() { + public void buildFieldDefinitionsUpdateActions_WithNewFieldDefinitionsAndNoOldFieldDefinitions_ShouldBuild3AddActions() { final Type oldType = mock(Type.class); when(oldType.getFieldDefinitions()).thenReturn(emptyList()); when(oldType.getKey()).thenReturn("type_key_1"); @@ -151,7 +151,7 @@ public void buildUpdateActions_WithNewFieldDefinitionsAndNoOldFieldDefinitions_S } @Test - public void buildUpdateActions_WithIdenticalFieldDefinitions_ShouldNotBuildUpdateActions() { + public void buildFieldDefinitionsUpdateActions_WithIdenticalFieldDefinitions_ShouldNotBuildUpdateActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -169,7 +169,7 @@ public void buildUpdateActions_WithIdenticalFieldDefinitions_ShouldNotBuildUpdat } @Test - public void buildUpdateActions_WithDuplicateFieldDefinitionNames_ShouldNotBuildActionsAndTriggerErrorCb() { + public void buildFieldDefinitionsUpdateActions_WithDuplicateFieldDefinitionNames_ShouldNotBuildActionsAndTriggerErrorCb() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -207,7 +207,7 @@ public void buildUpdateActions_WithDuplicateFieldDefinitionNames_ShouldNotBuildA } @Test - public void buildUpdateActions_WithOneMissingFieldDefinition_ShouldBuildRemoveFieldDefinitionAction() { + public void buildFieldDefinitionsUpdateActions_WithOneMissingFieldDefinition_ShouldBuildRemoveFieldDefinitionAction() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -225,7 +225,7 @@ public void buildUpdateActions_WithOneMissingFieldDefinition_ShouldBuildRemoveFi } @Test - public void buildUpdateActions_WithOneExtraFieldDefinition_ShouldBuildAddFieldDefinitionAction() { + public void buildFieldDefinitionsUpdateActions_WithOneExtraFieldDefinition_ShouldBuildAddFieldDefinitionAction() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -245,7 +245,7 @@ public void buildUpdateActions_WithOneExtraFieldDefinition_ShouldBuildAddFieldDe } @Test - public void buildUpdateActions_WithOneFieldDefinitionSwitch_ShouldBuildRemoveAndAddFieldDefinitionsActions() { + public void buildFieldDefinitionsUpdateActions_WithOneFieldDefinitionSwitch_ShouldBuildRemoveAndAddFieldDefinitionsActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -266,7 +266,7 @@ public void buildUpdateActions_WithOneFieldDefinitionSwitch_ShouldBuildRemoveAnd } @Test - public void buildUpdateActions_WithDifferent_ShouldBuildChangeFieldDefinitionOrderAction() { + public void buildFieldDefinitionsUpdateActions_WithDifferent_ShouldBuildChangeFieldDefinitionOrderAction() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -292,7 +292,7 @@ public void buildUpdateActions_WithDifferent_ShouldBuildChangeFieldDefinitionOrd } @Test - public void buildUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAndRemoveActions() { + public void buildFieldDefinitionsUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAndRemoveActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -318,7 +318,7 @@ public void buildUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrd } @Test - public void buildUpdateActions_WithAddedAndDifferentOrder_ShouldBuildChangeOrderAndAddActions() { + public void buildFieldDefinitionsUpdateActions_WithAddedAndDifferentOrder_ShouldBuildChangeOrderAndAddActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -346,7 +346,7 @@ public void buildUpdateActions_WithAddedAndDifferentOrder_ShouldBuildChangeOrder } @Test - public void buildUpdateActions_WithAddedFieldDefinitionInBetween_ShouldBuildChangeOrderAndAddActions() { + public void buildFieldDefinitionsUpdateActions_WithAddedFieldDefinitionInBetween_ShouldBuildChangeOrderAndAddActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -374,7 +374,7 @@ public void buildUpdateActions_WithAddedFieldDefinitionInBetween_ShouldBuildChan } @Test - public void buildUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAllThreeMoveFieldDefinitionActions() { + public void buildFieldDefinitionsUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAllThreeMoveFieldDefinitionActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -401,7 +401,7 @@ public void buildUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAllThreeMo } @Test - public void buildUpdateActions_WithDifferentType_ShouldRemoveOldFieldDefinitionAndAddNewFieldDefinition() { + public void buildFieldDefinitionsUpdateActions_WithDifferentType_ShouldRemoveOldFieldDefinitionAndAddNewFieldDefinition() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -421,7 +421,7 @@ public void buildUpdateActions_WithDifferentType_ShouldRemoveOldFieldDefinitionA } @Test - public void buildUpdateActions_WithNullNewFieldDefinition_ShouldSkipNullFieldDefinitions() { + public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinition_ShouldSkipNullFieldDefinitions() { // preparation final Type oldType = mock(Type.class); final FieldDefinition oldFieldDefinition = FieldDefinition.of( @@ -459,7 +459,7 @@ public void buildUpdateActions_WithNullNewFieldDefinition_ShouldSkipNullFieldDef } @Test - public void buildUpdateActions_WithDefinitionWithNullName_ShouldBuildChangeFieldDefinitionOrderAction() { + public void buildFieldDefinitionsUpdateActions_WithDefinitionWithNullName_ShouldBuildChangeFieldDefinitionOrderAction() { // preparation final Type oldType = mock(Type.class); final FieldDefinition oldFieldDefinition = FieldDefinition.of( From db1835ebaae778f9f97863976ae9ff436174cd3d Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 12:33:01 +0100 Subject: [PATCH 35/54] #300: Add unit test for fieldDefinition with null FieldType. --- ...BuildFieldDefinitionUpdateActionsTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java index 2d54000da7..5649d07b57 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java @@ -496,4 +496,41 @@ public void buildFieldDefinitionsUpdateActions_WithDefinitionWithNullName_Should RemoveFieldDefinition.of(oldFieldDefinition.getName()), AddFieldDefinition.of(newFieldDefinition)); } + + @Test + public void buildFieldDefinitionsUpdateActions_WithDefinitionWithNullType_ShouldBuildChangeFieldDefinitionOrderAction() { + // preparation + final Type oldType = mock(Type.class); + final FieldDefinition oldFieldDefinition = FieldDefinition.of( + LocalizedEnumFieldType.of(emptyList()), + "field_1", + LocalizedString.ofEnglish("label1"), + false, + TextInputHint.SINGLE_LINE); + + + when(oldType.getFieldDefinitions()).thenReturn(singletonList(oldFieldDefinition)); + + final FieldDefinition newFieldDefinition = FieldDefinition.of( + null, + "field_1", + LocalizedString.ofEnglish("label2"), + false, + TextInputHint.SINGLE_LINE); + + final TypeDraft typeDraft = TypeDraftBuilder + .of("key", LocalizedString.ofEnglish("label"), emptySet()) + .fieldDefinitions(asList(null, newFieldDefinition)) + .build(); + + // test + final List> updateActions = buildFieldDefinitionsUpdateActions( + oldType, + typeDraft, + SYNC_OPTIONS + ); + + // assertion + assertThat(updateActions).isEmpty(); + } } From dcb95f4bcd97248e05f6a9a256774c220637bfc6 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 13:12:59 +0100 Subject: [PATCH 36/54] #300: Unexpose enum differs. --- docs/RELEASE_NOTES.md | 3 --- .../commons/utils/EnumValuesUpdateActionUtils.java | 3 +++ .../utils/LocalizedEnumValueUpdateActionUtils.java | 12 ++++++++++-- .../utils/PlainEnumValueUpdateActionUtils.java | 14 ++++++++++++-- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/docs/RELEASE_NOTES.md b/docs/RELEASE_NOTES.md index 06436efe16..37d0761917 100644 --- a/docs/RELEASE_NOTES.md +++ b/docs/RELEASE_NOTES.md @@ -41,9 +41,6 @@ - **Type Sync** - Support for syncing types. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) For more info how to use it please refer to [Type usage doc](/docs/usage/TYPE_SYNC.md). - **Type Sync** - Exposed `TypeSyncUtils#buildActions` which calculates all needed update actions after comparing a `Type` and a `TypeDraft`. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - **Type Sync** - Exposed `TypeUpdateActionUtils` which contains utils for calculating needed update actions after comparing individual fields of a `Type` and a `TypeDraft`. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - - **Type Sync** - Exposed `LocalizedEnumValueUpdateActionUtils` which contains utils for calculating needed update actions after comparing two lists of `LocalizedEnumValue`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - - **Type Sync** - Exposed `PlainEnumValueUpdateActionUtils` which contains utils for calculating needed update actions after comparing two lists of `EnumValue`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - - **Commons** - Exposed `EnumValuesUpdateActionUtils` which contains generic utilities for calculating needed update actions after comparing two lists of `EnumValue`s or `LocalizedEnumValue`s. [#300](https://github.com/commercetools/commercetools-sync-java/issues/300) - 📋 **Documentation** (4) - **Commons** - Added the documentation github pages. https://commercetools.github.io/commercetools-sync-java diff --git a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java index 2e42900691..5b8ec9b808 100644 --- a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java @@ -26,6 +26,9 @@ import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.toMap; +/** + * This class is only meant for the internal use of the commercetools-sync-java library. + */ public final class EnumValuesUpdateActionUtils { /** diff --git a/src/main/java/com/commercetools/sync/types/utils/LocalizedEnumValueUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/LocalizedEnumValueUpdateActionUtils.java index be4651e740..c27577c3f2 100644 --- a/src/main/java/com/commercetools/sync/types/utils/LocalizedEnumValueUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/LocalizedEnumValueUpdateActionUtils.java @@ -13,7 +13,10 @@ import static com.commercetools.sync.commons.utils.EnumValuesUpdateActionUtils.buildActions; -public final class LocalizedEnumValueUpdateActionUtils { +/** + * This class is only meant for the internal use of the commercetools-sync-java library. + */ +final class LocalizedEnumValueUpdateActionUtils { /** * Compares a list of old {@link LocalizedEnumValue}s with a list of new {@link LocalizedEnumValue}s for a given * field definition and builds required update actions (e.g addLocalizedEnumValue, changeLocalizedEnumValueOrder). @@ -21,6 +24,11 @@ public final class LocalizedEnumValueUpdateActionUtils { * an empty {@link List} is returned. * *

+ * Note: Currently this util doesn't support the following: + *

    + *
  • removing the EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
  • updating the label of a EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
* TODO: Check GITHUB ISSUE#339 for missing FieldDefinition update actions. *

* @@ -32,7 +40,7 @@ public final class LocalizedEnumValueUpdateActionUtils { * @throws DuplicateKeyException in case there are localized enum values with duplicate keys. */ @Nonnull - public static List> buildLocalizedEnumValuesUpdateActions( + static List> buildLocalizedEnumValuesUpdateActions( @Nonnull final String fieldDefinitionName, @Nonnull final List oldEnumValues, @Nullable final List newEnumValues) { diff --git a/src/main/java/com/commercetools/sync/types/utils/PlainEnumValueUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/PlainEnumValueUpdateActionUtils.java index a17e206f14..4745530846 100644 --- a/src/main/java/com/commercetools/sync/types/utils/PlainEnumValueUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/PlainEnumValueUpdateActionUtils.java @@ -1,5 +1,6 @@ package com.commercetools.sync.types.utils; +import com.commercetools.sync.commons.exceptions.DuplicateKeyException; import com.commercetools.sync.commons.utils.EnumValuesUpdateActionUtils; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.models.EnumValue; @@ -11,7 +12,10 @@ import javax.annotation.Nullable; import java.util.List; -public final class PlainEnumValueUpdateActionUtils { +/** + * This class is only meant for the internal use of the commercetools-sync-java library. + */ +final class PlainEnumValueUpdateActionUtils { /** * Compares a list of old {@link EnumValue}s with a list of new {@link EnumValue}s for a given @@ -20,6 +24,11 @@ public final class PlainEnumValueUpdateActionUtils { * an empty {@link List} is returned. * *

+ * Note: Currently this util doesn't support the following: + *

    + *
  • removing the EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
  • updating the label of a EnumValue/LocalizedEnumValue of a FieldDefinition
    • + *
* TODO: Check GITHUB ISSUE#339 for missing FieldDefinition update actions. *

* @@ -28,9 +37,10 @@ public final class PlainEnumValueUpdateActionUtils { * @param newEnumValues the new list of plain enum values. * @return a list of plain enum values update actions if the list of plain enum values is not identical. * Otherwise, if the plain enum values are identical, an empty list is returned. + * @throws DuplicateKeyException in case there are localized enum values with duplicate keys. */ @Nonnull - public static List> buildEnumValuesUpdateActions( + static List> buildEnumValuesUpdateActions( @Nonnull final String fieldDefinitionName, @Nonnull final List oldEnumValues, @Nullable final List newEnumValues) { From b5bfa670c3c842b0c02cc994182048611468fd88 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 13:16:35 +0100 Subject: [PATCH 37/54] #300: Fix Javadocs. --- .../utils/AttributeDefinitionUpdateActionUtils.java | 3 +++ .../utils/AttributeDefinitionsUpdateActionUtils.java | 3 ++- .../utils/PlainEnumValueUpdateActionUtils.java | 2 ++ .../sync/types/utils/FieldDefinitionUpdateActionUtils.java | 7 ++++++- .../types/utils/FieldDefinitionsUpdateActionUtils.java | 4 +++- 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java b/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java index 3e95532455..59c83244c8 100644 --- a/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java @@ -1,5 +1,6 @@ package com.commercetools.sync.producttypes.utils; +import com.commercetools.sync.commons.exceptions.DuplicateKeyException; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.models.EnumValue; import io.sphere.sdk.models.LocalizedEnumValue; @@ -38,6 +39,8 @@ final class AttributeDefinitionUpdateActionUtils { * @param oldAttributeDefinition the old attribute definition which should be updated. * @param newAttributeDefinitionDraft the new attribute definition draft where we get the new fields. * @return A list with the update actions or an empty list if the attribute definition fields are identical. + * + * @throws DuplicateKeyException in case there are localized enum values with duplicate keys. */ @Nonnull static List> buildActions( diff --git a/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionsUpdateActionUtils.java b/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionsUpdateActionUtils.java index 5c3710eed9..f93ad00719 100644 --- a/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionsUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionsUpdateActionUtils.java @@ -53,7 +53,8 @@ final class AttributeDefinitionsUpdateActionUtils { * @param newAttributeDefinitionsDrafts the new list of attribute definitions drafts. * @return a list of attribute definitions update actions if the list of attribute definitions are not identical. * Otherwise, if the attribute definitions are identical, an empty list is returned. - * @throws DuplicateNameException in case there are attribute definitions drafts with duplicate names. + * @throws BuildUpdateActionException in case there are attribute definitions drafts with duplicate names or enums + * duplicate keys. */ @Nonnull static List> buildAttributeDefinitionsUpdateActions( diff --git a/src/main/java/com/commercetools/sync/producttypes/utils/PlainEnumValueUpdateActionUtils.java b/src/main/java/com/commercetools/sync/producttypes/utils/PlainEnumValueUpdateActionUtils.java index 375985b1e3..6fd0ce0838 100644 --- a/src/main/java/com/commercetools/sync/producttypes/utils/PlainEnumValueUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/producttypes/utils/PlainEnumValueUpdateActionUtils.java @@ -1,5 +1,6 @@ package com.commercetools.sync.producttypes.utils; +import com.commercetools.sync.commons.exceptions.DuplicateKeyException; import com.commercetools.sync.commons.utils.EnumValuesUpdateActionUtils; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.models.EnumValue; @@ -34,6 +35,7 @@ public final class PlainEnumValueUpdateActionUtils { * @param newEnumValues the new list of plain enum values. * @return a list of plain enum values update actions if the list of plain enum values is not identical. * Otherwise, if the plain enum values are identical, an empty list is returned. + * @throws DuplicateKeyException in case there are localized enum values with duplicate keys. */ @Nonnull public static List> buildEnumValuesUpdateActions( diff --git a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java index 59fafc312b..a558724285 100644 --- a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java @@ -1,5 +1,6 @@ package com.commercetools.sync.types.utils; +import com.commercetools.sync.commons.exceptions.DuplicateKeyException; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.models.EnumValue; import io.sphere.sdk.models.LocalizedEnumValue; @@ -26,7 +27,7 @@ final class FieldDefinitionUpdateActionUtils { /** - * Compares all the fields of old {@link FieldDefinition} with new {@link FieldDefinition} and returns + * Compares all the fields of an old {@link FieldDefinition} with a new {@link FieldDefinition} and returns * a list of {@link UpdateAction}<{@link Type}> as a result. If both the {@link FieldDefinition} * and the {@link FieldDefinition} have identical fields, then no update action is needed and hence an * empty {@link List} is returned. @@ -34,6 +35,8 @@ final class FieldDefinitionUpdateActionUtils { * @param oldFieldDefinition the old field definition which should be updated. * @param newFieldDefinition the new field definition where we get the new fields. * @return A list with the update actions or an empty list if the field definition fields are identical. + * + * @throws DuplicateKeyException in case there are localized enum values with duplicate keys. */ @Nonnull static List> buildActions( @@ -56,6 +59,8 @@ static List> buildActions( * @param oldFieldDefinition the old field definition which should be updated. * @param newFieldDefinition the new field definition where we get the new fields. * @return A list with the update actions or an empty list if the field definition enums are identical. + * + * @throws DuplicateKeyException in case there are localized enum values with duplicate keys. */ @Nonnull static List> buildEnumUpdateActions( diff --git a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java index 8c8bb259a9..5146813cd6 100644 --- a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java @@ -53,7 +53,8 @@ final class FieldDefinitionsUpdateActionUtils { * @param newFieldDefinitions the new list of field definitions. * @return a list of field definitions update actions if the list of field definitions is not identical. * Otherwise, if the field definitions are identical, an empty list is returned. - * @throws DuplicateNameException in case there are field definitions with duplicate names. + * @throws BuildUpdateActionException in case there are field definitions with duplicate names or enums + * duplicate keys. */ @Nonnull static List> buildFieldDefinitionsUpdateActions( @@ -128,6 +129,7 @@ private static List> buildUpdateActions( * definition fields (name, label, etc..), and add the computed actions to the list of update actions. * Otherwise, if the field definitions are identical, an empty optional is returned. * @throws DuplicateNameException in case there are field definitions drafts with duplicate names. + * @throws DuplicateKeyException in case there are enum values with duplicate keys. */ @Nonnull private static List> buildRemoveFieldDefinitionOrFieldDefinitionUpdateActions( From bbaab0114164f8af20a8f3f8e4e49090b8fe36c8 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 13:16:47 +0100 Subject: [PATCH 38/54] #300: Add more readable line spacing. --- .../utils/AttributeDefinitionUpdateActionUtils.java | 1 + .../sync/types/utils/FieldDefinitionUpdateActionUtils.java | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java b/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java index 59c83244c8..7eb47b7daf 100644 --- a/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/producttypes/utils/AttributeDefinitionUpdateActionUtils.java @@ -75,6 +75,7 @@ private static List> buildEnumUpdateActions( @Nonnull final AttributeDefinitionDraft newAttributeDefinitionDraft) { final List> updateActions = new ArrayList<>(); + if (isPlainEnumAttribute(oldAttributeDefinition)) { updateActions.addAll(buildEnumValuesUpdateActions( oldAttributeDefinition.getName(), diff --git a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java index a558724285..50ec8f6f7b 100644 --- a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java @@ -68,18 +68,23 @@ static List> buildEnumUpdateActions( @Nonnull final FieldDefinition newFieldDefinition) { final List> updateActions = new ArrayList<>(); + if (isPlainEnumField(oldFieldDefinition)) { + updateActions.addAll(buildEnumValuesUpdateActions( oldFieldDefinition.getName(), ((EnumFieldType) oldFieldDefinition.getType()).getValues(), ((EnumFieldType) newFieldDefinition.getType()).getValues() )); + } else if (isLocalizedEnumField(oldFieldDefinition)) { + updateActions.addAll(buildLocalizedEnumValuesUpdateActions( oldFieldDefinition.getName(), ((LocalizedEnumFieldType) oldFieldDefinition.getType()).getValues(), ((LocalizedEnumFieldType) newFieldDefinition.getType()).getValues() )); + } return updateActions; } From a87995df787317bfa2dd1d7c770efba24da30721 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 13:18:41 +0100 Subject: [PATCH 39/54] #300: Move tests to accessible package. --- .../BuildFieldDefinitionUpdateActionsTest.java | 2 +- .../BuildLocalizedEnumUpdateActionsTest.java | 2 +- .../{typeactionutils => }/BuildPlainEnumUpdateActionsTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/test/java/com/commercetools/sync/types/utils/{typeactionutils => }/BuildFieldDefinitionUpdateActionsTest.java (99%) rename src/test/java/com/commercetools/sync/types/utils/{typeactionutils => }/BuildLocalizedEnumUpdateActionsTest.java (99%) rename src/test/java/com/commercetools/sync/types/utils/{typeactionutils => }/BuildPlainEnumUpdateActionsTest.java (98%) diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java similarity index 99% rename from src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java rename to src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java index 5649d07b57..1a72f9c408 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java @@ -1,4 +1,4 @@ -package com.commercetools.sync.types.utils.typeactionutils; +package com.commercetools.sync.types.utils; import com.commercetools.sync.commons.exceptions.BuildUpdateActionException; import com.commercetools.sync.commons.exceptions.DuplicateNameException; diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildLocalizedEnumUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java similarity index 99% rename from src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildLocalizedEnumUpdateActionsTest.java rename to src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java index bd4093d422..7b97c8538b 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildLocalizedEnumUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java @@ -1,4 +1,4 @@ -package com.commercetools.sync.types.utils.typeactionutils; +package com.commercetools.sync.types.utils; import com.commercetools.sync.commons.exceptions.DuplicateKeyException; import com.commercetools.sync.types.utils.LocalizedEnumValueUpdateActionUtils; diff --git a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildPlainEnumUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java similarity index 98% rename from src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildPlainEnumUpdateActionsTest.java rename to src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java index 2821ecb2b9..d474c591b9 100644 --- a/src/test/java/com/commercetools/sync/types/utils/typeactionutils/BuildPlainEnumUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java @@ -1,4 +1,4 @@ -package com.commercetools.sync.types.utils.typeactionutils; +package com.commercetools.sync.types.utils; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.types.Type; From 5bbf06d9ec049db12f418de016fe90f522985e79 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 14:53:58 +0100 Subject: [PATCH 40/54] #300: Unexpose generic enum differ. --- .../commons/utils/EnumValuesUpdateActionUtils.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java index 5b8ec9b808..62821b64da 100644 --- a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java @@ -27,7 +27,7 @@ import static java.util.stream.Collectors.toMap; /** - * This class is only meant for the internal use of the commercetools-sync-java library. + * The utils in this class are only meant for the internal use of the commercetools-sync-java library. */ public final class EnumValuesUpdateActionUtils { @@ -222,7 +222,7 @@ private static List> getRemoveEnumValuesU * Otherwise, if the enum values order is identical, an empty optional is returned. */ @Nonnull - public static Optional> buildChangeEnumValuesOrderUpdateAction( + static Optional> buildChangeEnumValuesOrderUpdateAction( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, @@ -250,7 +250,7 @@ public static Optional> buildChangeEnumVa * Otherwise, if the enum values order is identical, an empty optional is returned. */ @Nonnull - public static Optional> buildChangeEnumValuesWithKeysOrderUpdateAction( + private static Optional> buildChangeEnumValuesWithKeysOrderUpdateAction( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, @@ -306,7 +306,7 @@ private static Pair, List> getAllKeysAn * Otherwise, if the enum values are identical, an empty optional is returned. */ @Nonnull - public static List> buildAddEnumValuesUpdateActions( + static List> buildAddEnumValuesUpdateActions( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, @@ -344,7 +344,7 @@ public static List> buildAddEnumValuesUpd * Otherwise, if the enum values are identical, an empty optional is returned. */ @Nonnull - public static Optional> buildRemoveEnumValuesUpdateAction( + static Optional> buildRemoveEnumValuesUpdateAction( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nullable final List newEnumValues, @@ -383,7 +383,7 @@ public static Optional> buildRemoveEnumVa * is returned. */ @Nonnull - public static List> buildMatchingEnumValuesUpdateActions( + static List> buildMatchingEnumValuesUpdateActions( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, From 2459b38f49fa9a997304e2badfa08a26f90413f4 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 14:55:47 +0100 Subject: [PATCH 41/54] #300: Remove redundant empty check. --- .../sync/commons/utils/EnumValuesUpdateActionUtils.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java index 62821b64da..53d2a7bbfd 100644 --- a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java @@ -62,7 +62,7 @@ public static List> buildActions( @Nullable final BiFunction, UpdateAction> changeOrderEnumCallback, @Nullable final BiFunction, UpdateAction> changeOrderWithKeysEnumCallback) { - if (newEnumValues != null && !newEnumValues.isEmpty()) { + if (newEnumValues != null) { return buildUpdateActions(definitionName, oldEnumValues, newEnumValues, @@ -76,11 +76,10 @@ public static List> buildActions( return buildRemoveEnumValuesUpdateAction( definitionName, oldEnumValues, - newEnumValues, + null, removeEnumCallback) .map(Collections::singletonList) .orElse(emptyList()); - } return emptyList(); From f6e34d2140d9b37781e32703b1ae661a16f0b5ce Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 14:56:16 +0100 Subject: [PATCH 42/54] #300: Replace singleton lists with optionals. --- .../utils/EnumValuesUpdateActionUtils.java | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java index 53d2a7bbfd..f3c92457af 100644 --- a/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/commons/utils/EnumValuesUpdateActionUtils.java @@ -96,8 +96,8 @@ private static List> buildUpdateActions( @Nullable final BiFunction, UpdateAction> changeOrderEnumCallback, @Nullable final BiFunction, UpdateAction> changeOrderWithKeysEnumCallback) { - final List> removeEnumValuesUpdateActions = - getRemoveEnumValuesUpdateActions(definitionName, oldEnumValues, newEnumValues, + final Optional> removeEnumValuesUpdateAction = + getRemoveEnumValuesUpdateAction(definitionName, oldEnumValues, newEnumValues, removeEnumValuesUpdateActionCallback); final List> matchingEnumValuesUpdateActions = getMatchingEnumValuesUpdateActions( @@ -106,56 +106,55 @@ private static List> buildUpdateActions( final List> addEnumValuesUpdateActions = getAddEnumValuesUpdateActions(definitionName, oldEnumValues, newEnumValues, addEnumCallback); - final List> changeEnumValuesOrderUpdateActions = getChangeEnumValuesOrderUpdateActions( + final Optional> changeEnumValuesOrderUpdateAction = getChangeEnumValuesOrderUpdateAction( definitionName, oldEnumValues, newEnumValues, changeOrderEnumCallback); - final List> changeEnumValuesWithKeysOrderUpdateActions = - getChangeEnumValuesWithKeysOrderUpdateActions( + final Optional> changeEnumValuesWithKeysOrderUpdateActions = + getChangeEnumValuesWithKeysOrderUpdateAction( definitionName, oldEnumValues, newEnumValues, changeOrderWithKeysEnumCallback); - return Stream.of(removeEnumValuesUpdateActions, + return Stream + .of( + removeEnumValuesUpdateAction.map(Collections::singletonList).orElse(emptyList()), matchingEnumValuesUpdateActions, addEnumValuesUpdateActions, - changeEnumValuesOrderUpdateActions, - changeEnumValuesWithKeysOrderUpdateActions) - .flatMap(Collection::stream) - .collect(Collectors.toList()); + changeEnumValuesOrderUpdateAction.map(Collections::singletonList).orElse(emptyList()), + changeEnumValuesWithKeysOrderUpdateActions.map(Collections::singletonList).orElse(emptyList())) + .flatMap(Collection::stream) + .collect(Collectors.toList()); } @Nonnull - private static List> getChangeEnumValuesWithKeysOrderUpdateActions( + private static Optional> getChangeEnumValuesWithKeysOrderUpdateAction( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, @Nullable final BiFunction, UpdateAction> changeOrderWithKeysEnumCallback) { - return changeOrderWithKeysEnumCallback == null ? emptyList() : + return changeOrderWithKeysEnumCallback == null ? Optional.empty() : buildChangeEnumValuesWithKeysOrderUpdateAction( definitionName, oldEnumValues, newEnumValues, changeOrderWithKeysEnumCallback - ).map(Collections::singletonList) - .orElse(emptyList()); + ); } @Nonnull - private static List> getChangeEnumValuesOrderUpdateActions( + private static Optional> getChangeEnumValuesOrderUpdateAction( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, @Nullable final BiFunction, UpdateAction> changeOrderEnumCallback) { - return changeOrderEnumCallback == null ? emptyList() : + return changeOrderEnumCallback == null ? Optional.empty() : buildChangeEnumValuesOrderUpdateAction( - definitionName, - oldEnumValues, - newEnumValues, - changeOrderEnumCallback - ) - .map(Collections::singletonList) - .orElse(emptyList()); + definitionName, + oldEnumValues, + newEnumValues, + changeOrderEnumCallback + ); } @Nonnull @@ -190,20 +189,18 @@ private static List> getMatchingEnumValue } @Nonnull - private static List> getRemoveEnumValuesUpdateActions( + private static Optional> getRemoveEnumValuesUpdateAction( @Nonnull final String definitionName, @Nonnull final List oldEnumValues, @Nonnull final List newEnumValues, @Nullable final BiFunction, UpdateAction> removeEnumValuesUpdateActionCallback) { - return removeEnumValuesUpdateActionCallback == null ? emptyList() : + return removeEnumValuesUpdateActionCallback == null ? Optional.empty() : buildRemoveEnumValuesUpdateAction( definitionName, oldEnumValues, newEnumValues, - removeEnumValuesUpdateActionCallback) - .map(Collections::singletonList) - .orElse(emptyList()); + removeEnumValuesUpdateActionCallback); } /** From 4ab3f1d9f79a61b908b00fce6fe24f1419ef1b69 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 14:59:04 +0100 Subject: [PATCH 43/54] #300: Change test names. --- .../types/utils/BuildLocalizedEnumUpdateActionsTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java index 7b97c8538b..292d7036ad 100644 --- a/src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildLocalizedEnumUpdateActionsTest.java @@ -1,7 +1,6 @@ package com.commercetools.sync.types.utils; import com.commercetools.sync.commons.exceptions.DuplicateKeyException; -import com.commercetools.sync.types.utils.LocalizedEnumValueUpdateActionUtils; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.types.Type; import io.sphere.sdk.types.commands.updateactions.AddLocalizedEnumValue; @@ -25,7 +24,7 @@ public class BuildLocalizedEnumUpdateActionsTest { public ExpectedException expectedException = ExpectedException.none(); @Test - public void buildLocalizedEnumUpdateActions_WithEmptyPlainEnumValuesAndNoOlEnumValues_ShouldNotBuildActions() { + public void buildLocalizedEnumUpdateActions_WithEmptyPlainEnumValuesAndNoOldEnumValues_ShouldNotBuildActions() { final List> updateActions = buildLocalizedEnumValuesUpdateActions( FIELD_NAME_1, emptyList(), @@ -179,7 +178,7 @@ public void buildLocalizedEnumUpdateActions_WithAddedRemovedAndDifOrder_ShouldBu } @Test - public void buildLocalizedEnumUpdateActions_WithDuplicatePlainEnumValues_ShouldTriggerDuplicateKeyError() { + public void buildLocalizedEnumUpdateActions_WithDuplicateEnumValues_ShouldTriggerDuplicateKeyError() { expectedException.expect(DuplicateKeyException.class); expectedException.expectMessage("Enum Values have duplicated keys. Definition name: " + "'field_definition_name', Duplicated enum value: 'b'. Enum Values are expected to be unique inside " From f84764fd0f495917622cec1afc20267fc14bf14f Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 14:59:42 +0100 Subject: [PATCH 44/54] #300: Fix unit tests. --- .../BuildPlainEnumUpdateActionsTest.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java index d474c591b9..51417c0011 100644 --- a/src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildPlainEnumUpdateActionsTest.java @@ -1,10 +1,13 @@ package com.commercetools.sync.types.utils; +import com.commercetools.sync.commons.exceptions.DuplicateKeyException; import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.types.Type; import io.sphere.sdk.types.commands.updateactions.AddEnumValue; import io.sphere.sdk.types.commands.updateactions.ChangeEnumValueOrder; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import java.util.List; @@ -17,8 +20,11 @@ public class BuildPlainEnumUpdateActionsTest { private static final String FIELD_NAME_1 = "field1"; + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Test - public void buildPlainEnumUpdateActions_WithEmptyPlainEnumValuesAndNoOlEnumValues_ShouldNotBuildActions() { + public void buildPlainEnumUpdateActions_WithEmptyPlainEnumValuesAndNoOldEnumValues_ShouldNotBuildActions() { final List> updateActions = buildEnumValuesUpdateActions( FIELD_NAME_1, emptyList(), @@ -68,7 +74,7 @@ public void buildPlainEnumUpdateActions_WithOnePlainEnumValue_ShouldBuildAddEnum } @Test - public void buildPlainEnumUpdateActions_WithOneEnumValueSwitch_ShouldBuildRemoveAndAddEnumValueActions() { + public void buildPlainEnumUpdateActions_WithOneEnumValueSwitch_ShouldBuildAddEnumValueActions() { final List> updateActions = buildEnumValuesUpdateActions( FIELD_NAME_1, ENUM_VALUES_ABC, @@ -81,7 +87,7 @@ public void buildPlainEnumUpdateActions_WithOneEnumValueSwitch_ShouldBuildRemove } @Test - public void buildPlainEnumUpdateActions_WithDifferent_ShouldBuildChangeEnumValueOrderAction() { + public void buildPlainEnumUpdateActions_WithDifferentOrder_ShouldBuildChangeEnumValueOrderAction() { final List> updateActions = buildEnumValuesUpdateActions( FIELD_NAME_1, ENUM_VALUES_ABC, @@ -98,7 +104,7 @@ public void buildPlainEnumUpdateActions_WithDifferent_ShouldBuildChangeEnumValue } @Test - public void buildPlainEnumUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAndRemoveActions() { + public void buildPlainEnumUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAction() { final List> updateActions = buildEnumValuesUpdateActions( FIELD_NAME_1, ENUM_VALUES_ABC, @@ -152,7 +158,7 @@ public void buildPlainEnumUpdateActions_WithAddedEnumValueInBetween_ShouldBuildC } @Test - public void buildPlainEnumUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAllThreeMoveEnumValueActions() { + public void buildPlainEnumUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAddAndChangeOrderActions() { final List> updateActions = buildEnumValuesUpdateActions( FIELD_NAME_1, ENUM_VALUES_ABC, @@ -168,4 +174,18 @@ public void buildPlainEnumUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildA )) ); } + + @Test + public void buildLocalizedEnumUpdateActions_WithDuplicateEnumValues_ShouldTriggerDuplicateKeyError() { + expectedException.expect(DuplicateKeyException.class); + expectedException.expectMessage("Enum Values have duplicated keys. Definition name: " + + "'field_definition_name', Duplicated enum value: 'b'. Enum Values are expected to be unique inside " + + "their definition."); + + buildEnumValuesUpdateActions( + "field_definition_name", + ENUM_VALUES_ABC, + ENUM_VALUES_ABB + ); + } } From a4ecf2d73ef17ceecb024009d071429cc25f0505 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:02:11 +0100 Subject: [PATCH 45/54] #300: Use query builder. --- .../sync/services/impl/ProductTypeServiceImpl.java | 2 +- .../com/commercetools/sync/services/impl/TypeServiceImpl.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commercetools/sync/services/impl/ProductTypeServiceImpl.java b/src/main/java/com/commercetools/sync/services/impl/ProductTypeServiceImpl.java index 010b33751e..b310fbb319 100644 --- a/src/main/java/com/commercetools/sync/services/impl/ProductTypeServiceImpl.java +++ b/src/main/java/com/commercetools/sync/services/impl/ProductTypeServiceImpl.java @@ -154,7 +154,7 @@ public CompletionStage> fetchProductType(@Nullable final S } final ProductTypeQuery productTypeQuery = - ProductTypeQuery.of().plusPredicates(queryModel -> queryModel.key().is(key)); + ProductTypeQueryBuilder.of().plusPredicates(queryModel -> queryModel.key().is(key)).build(); return syncOptions .getCtpClient() diff --git a/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java b/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java index 77d96a7901..a7e58ad768 100644 --- a/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java +++ b/src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java @@ -58,6 +58,7 @@ private CompletionStage> fetchAndCache(@Nonnull final String ke @Nonnull @Override public CompletionStage> fetchMatchingTypesByKeys(@Nonnull final Set keys) { + if (keys.isEmpty()) { return CompletableFuture.completedFuture(Collections.emptySet()); } @@ -84,7 +85,7 @@ public CompletionStage> fetchType(@Nullable final String key) { } final TypeQuery typeQuery = - TypeQuery.of().plusPredicates(queryModel -> queryModel.key().is(key)); + TypeQueryBuilder.of().plusPredicates(queryModel -> queryModel.key().is(key)).build(); return syncOptions .getCtpClient() From 0c1e47f24b5eb8a230931d68ca1ba037545e443b Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:02:21 +0100 Subject: [PATCH 46/54] #300: Fix Javadocs. --- .../commercetools/sync/services/ProductTypeService.java | 2 +- .../java/com/commercetools/sync/services/TypeService.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/commercetools/sync/services/ProductTypeService.java b/src/main/java/com/commercetools/sync/services/ProductTypeService.java index d394fe6f5f..2e4b669913 100644 --- a/src/main/java/com/commercetools/sync/services/ProductTypeService.java +++ b/src/main/java/com/commercetools/sync/services/ProductTypeService.java @@ -73,7 +73,7 @@ CompletionStage>> fetchCachedProductAttr * Given a resource draft of type {@link ProductTypeDraft}, this method attempts to create a resource * {@link ProductType} based on it in the CTP project defined by the sync options. * - *

A completion stage containing an empty option and the error callback will be triggered in those cases: + *

A completion stage containing an empty optional and the error callback will be triggered in those cases: *

    *
  • the draft has a blank key
    • *
  • the create request fails on CTP
    • diff --git a/src/main/java/com/commercetools/sync/services/TypeService.java b/src/main/java/com/commercetools/sync/services/TypeService.java index 956e894b0c..2ebce77ad7 100644 --- a/src/main/java/com/commercetools/sync/services/TypeService.java +++ b/src/main/java/com/commercetools/sync/services/TypeService.java @@ -52,7 +52,7 @@ public interface TypeService { * returned in the returned future. * * @param key the key of the type to fetch. - * @return {@link CompletionStage}<{@link Optional}> in which the result of it's completion contains an + * @return {@link CompletionStage}<{@link Optional}> in which the result of its completion contains an * {@link Optional} that contains the matching {@link Type} if exists, otherwise empty. */ @Nonnull @@ -62,14 +62,14 @@ public interface TypeService { * Given a resource draft of type {@link TypeDraft}, this method attempts to create a resource * {@link Type} based on it in the CTP project defined by the sync options. * - *

    A completion stage containing an empty option and the error callback will be triggered in those cases: + *

    A completion stage containing an empty optional and the error callback will be triggered in those cases: *

      *
    • the draft has a blank key
      • *
    • the create request fails on CTP
      • *
    * *

    On the other hand, if the resource gets created successfully on CTP, then the created resource's id and - * key are cached and the method returns a {@link CompletionStage} in which the result of it's completion + * key are cached and the method returns a {@link CompletionStage} in which the result of its completion * contains an instance {@link Optional} of the resource which was created. * * @param typeDraft the resource draft to create a resource based off of. @@ -83,7 +83,7 @@ public interface TypeService { * Given a {@link Type} and a {@link List}<{@link UpdateAction}<{@link Type}>>, this method * issues an update request with these update actions on this {@link Type} in the CTP project defined in an * injected {@link io.sphere.sdk.client.SphereClient}. This method returns - * {@link CompletionStage}<{@link Type}> in which the result of it's completion contains an instance of + * {@link CompletionStage}<{@link Type}> in which the result of its completion contains an instance of * the {@link Type} which was updated in the CTP project. * * @param type the {@link Type} to update. From 24dca2c729b045832f51f788586f6a59177fbcd3 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:47:22 +0100 Subject: [PATCH 47/54] #300: Change fixtures class name, package and contents. --- .../sync/types/FieldDefinitionTestHelper.java | 69 ------------------- .../types/utils/FieldDefinitionFixtures.java | 35 ++++++++++ 2 files changed, 35 insertions(+), 69 deletions(-) delete mode 100644 src/test/java/com/commercetools/sync/types/FieldDefinitionTestHelper.java create mode 100644 src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java diff --git a/src/test/java/com/commercetools/sync/types/FieldDefinitionTestHelper.java b/src/test/java/com/commercetools/sync/types/FieldDefinitionTestHelper.java deleted file mode 100644 index c2c2b730e6..0000000000 --- a/src/test/java/com/commercetools/sync/types/FieldDefinitionTestHelper.java +++ /dev/null @@ -1,69 +0,0 @@ -package com.commercetools.sync.types; - -import io.sphere.sdk.categories.Category; -import io.sphere.sdk.models.EnumValue; -import io.sphere.sdk.models.LocalizedString; -import io.sphere.sdk.models.TextInputHint; -import io.sphere.sdk.types.EnumFieldType; -import io.sphere.sdk.types.FieldDefinition; -import io.sphere.sdk.types.LocalizedStringFieldType; -import io.sphere.sdk.types.ReferenceFieldType; -import io.sphere.sdk.types.SetFieldType; -import io.sphere.sdk.types.StringFieldType; - -import java.util.Arrays; -import java.util.List; - -public final class FieldDefinitionTestHelper { - - public static FieldDefinition stringFieldDefinition(final String fieldName, - final String labelEng, - boolean required, - final TextInputHint hint) { - return FieldDefinition.of(StringFieldType.of(), - fieldName, - LocalizedString.ofEnglish(labelEng), - required, - hint); - } - - public static FieldDefinition localizedStringFieldDefinition(final String fieldName, - final String labelEng, - boolean required, - final TextInputHint hint) { - return FieldDefinition.of(LocalizedStringFieldType.of(), - fieldName, - LocalizedString.ofEnglish(labelEng), - required, - hint); - } - - public static FieldDefinition stateFieldDefinition() { - final List values = Arrays.asList( - EnumValue.of("published", "the category is publicly visible"), - EnumValue.of("draft", "the category should not be displayed in the frontend") - ); - final boolean required = false; - final LocalizedString label = LocalizedString.ofEnglish("state of the category concerning to show it publicly"); - final String fieldName = "state"; - return FieldDefinition - .of(EnumFieldType.of(values), fieldName, label, required); - } - - public static FieldDefinition imageUrlFieldDefinition() { - final LocalizedString imageUrlLabel = - LocalizedString.ofEnglish("absolute url to an image to display for the category"); - return FieldDefinition - .of(StringFieldType.of(), "imageUrl", imageUrlLabel, false, TextInputHint.SINGLE_LINE); - } - - public static FieldDefinition relatedCategoriesFieldDefinition() { - final LocalizedString relatedCategoriesLabel = - LocalizedString.ofEnglish("categories to suggest products similar to the current category"); - //referenceTypeId is required to refere to categories - final String referenceTypeId = Category.referenceTypeId(); - final SetFieldType setType = SetFieldType.of(ReferenceFieldType.of(referenceTypeId)); - return FieldDefinition - .of(setType, "relatedCategories", relatedCategoriesLabel, false); - } -} diff --git a/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java b/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java new file mode 100644 index 0000000000..da3189bd6b --- /dev/null +++ b/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java @@ -0,0 +1,35 @@ +package com.commercetools.sync.types; + +import io.sphere.sdk.models.LocalizedString; +import io.sphere.sdk.models.TextInputHint; +import io.sphere.sdk.types.FieldDefinition; +import io.sphere.sdk.types.LocalizedStringFieldType; +import io.sphere.sdk.types.StringFieldType; + +public final class FieldDefinitionTestHelper { + + public static FieldDefinition stringFieldDefinition(final String fieldName, + final String labelEng, + boolean required, + final TextInputHint hint) { + return FieldDefinition.of(StringFieldType.of(), + fieldName, + LocalizedString.ofEnglish(labelEng), + required, + hint); + } + + public static FieldDefinition localizedStringFieldDefinition(final String fieldName, + final String labelEng, + boolean required, + final TextInputHint hint) { + return FieldDefinition.of(LocalizedStringFieldType.of(), + fieldName, + LocalizedString.ofEnglish(labelEng), + required, + hint); + } + + private FieldDefinitionTestHelper() { + } +} From 7f6912c3effa0ff0a9c247c25a7d4b1c0a2cb53c Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:47:40 +0100 Subject: [PATCH 48/54] #300: Avoid star import for FieldDefinitionFixtures. --- config/checkstyle/checkstyle.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml index 27cd1caf37..f1b87f28f3 100644 --- a/config/checkstyle/checkstyle.xml +++ b/config/checkstyle/checkstyle.xml @@ -72,7 +72,8 @@ + com.commercetools.sync.commons.utils.PlainEnumValueFixtures, + com.commercetools.sync.types.utils.FieldDefinitionFixtures"/> From 9c96839e9b4021dfe7fe5b1bc26fbbe79e222e5a Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:48:17 +0100 Subject: [PATCH 49/54] #300: Add unit tests for mixed cases. --- .../utils/LocalizedEnumValueFixtures.java | 4 ++++ .../commons/utils/PlainEnumValueFixtures.java | 2 ++ .../BuildLocalizedEnumUpdateActionsTest.java | 19 +++++++++++++++ .../BuildPlainEnumUpdateActionsTest.java | 24 +++++++++++++++---- 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/test/java/com/commercetools/sync/commons/utils/LocalizedEnumValueFixtures.java b/src/test/java/com/commercetools/sync/commons/utils/LocalizedEnumValueFixtures.java index 02daedaeed..8615a5d8f3 100644 --- a/src/test/java/com/commercetools/sync/commons/utils/LocalizedEnumValueFixtures.java +++ b/src/test/java/com/commercetools/sync/commons/utils/LocalizedEnumValueFixtures.java @@ -19,6 +19,10 @@ public final class LocalizedEnumValueFixtures { public static final List ENUM_VALUES_ABC = unmodifiableList(asList(ENUM_VALUE_A, ENUM_VALUE_B, ENUM_VALUE_C)); + public static final List ENUM_VALUES_BAC = + unmodifiableList(asList(ENUM_VALUE_B, ENUM_VALUE_A, ENUM_VALUE_C)); + public static final List ENUM_VALUES_AB_WITH_DIFFERENT_LABEL = + unmodifiableList(asList(ENUM_VALUE_A_DIFFERENT_LABEL, ENUM_VALUE_B)); public static final List ENUM_VALUES_AB = unmodifiableList(asList(ENUM_VALUE_A, ENUM_VALUE_B)); public static final List ENUM_VALUES_ABB = diff --git a/src/test/java/com/commercetools/sync/commons/utils/PlainEnumValueFixtures.java b/src/test/java/com/commercetools/sync/commons/utils/PlainEnumValueFixtures.java index 297062a83c..a9c8c07c68 100644 --- a/src/test/java/com/commercetools/sync/commons/utils/PlainEnumValueFixtures.java +++ b/src/test/java/com/commercetools/sync/commons/utils/PlainEnumValueFixtures.java @@ -19,6 +19,8 @@ public final class PlainEnumValueFixtures { unmodifiableList(asList(ENUM_VALUE_A, ENUM_VALUE_B, ENUM_VALUE_C)); public static final List ENUM_VALUES_AB = unmodifiableList(asList(ENUM_VALUE_A, ENUM_VALUE_B)); + public static final List ENUM_VALUES_BAC = + unmodifiableList(asList(ENUM_VALUE_B, ENUM_VALUE_A, ENUM_VALUE_C)); public static final List ENUM_VALUES_AB_WITH_DIFFERENT_LABEL = unmodifiableList(asList(ENUM_VALUE_A_DIFFERENT_LABEL, ENUM_VALUE_B)); public static final List ENUM_VALUES_ABB = diff --git a/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildLocalizedEnumUpdateActionsTest.java b/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildLocalizedEnumUpdateActionsTest.java index 0538364130..4fe65ae4dd 100644 --- a/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildLocalizedEnumUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildLocalizedEnumUpdateActionsTest.java @@ -160,6 +160,25 @@ public void buildLocalizedEnumUpdateActions_WithDifferent_ShouldBuildChangeEnumV ); } + @Test + public void buildPlainEnumUpdateActions_WithMixedCase_ShouldBuildChangeEnumValueOrderAction() { + final String attributeDefinitionName = "attribute_definition_name_1"; + final List> updateActions = buildLocalizedEnumValuesUpdateActions( + attributeDefinitionName, + ENUM_VALUES_BAC, + ENUM_VALUES_AB_WITH_DIFFERENT_LABEL + ); + + assertThat(updateActions).containsExactly( + RemoveEnumValues.of(attributeDefinitionName, ENUM_VALUE_C.getKey()), + ChangeLocalizedEnumValueLabel.of(attributeDefinitionName, ENUM_VALUE_A_DIFFERENT_LABEL), + ChangeLocalizedEnumValueOrder.of(attributeDefinitionName, asList( + ENUM_VALUE_A_DIFFERENT_LABEL, + ENUM_VALUE_B + )) + ); + } + @Test public void buildLocalizedEnumUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAndRemoveActions() { final List> updateActions = buildLocalizedEnumValuesUpdateActions( diff --git a/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildPlainEnumUpdateActionsTest.java b/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildPlainEnumUpdateActionsTest.java index df54878965..79bdb75e70 100644 --- a/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildPlainEnumUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/producttypes/utils/producttypeactionutils/BuildPlainEnumUpdateActionsTest.java @@ -89,7 +89,6 @@ public void buildPlainEnumUpdateActions_WithIdenticalPlainEnum_ShouldNotBuildUpd @Rule public ExpectedException expectedException = ExpectedException.none(); - @Test public void buildPlainEnumUpdateActions_WithDuplicatePlainEnumValues_ShouldTriggerDuplicateKeyError() { expectedException.expect(DuplicateKeyException.class); @@ -104,7 +103,6 @@ public void buildPlainEnumUpdateActions_WithDuplicatePlainEnumValues_ShouldTrigg ); } - @Test public void buildPlainEnumUpdateActions_WithOneMissingPlainEnumValue_ShouldBuildRemoveEnumValueAction() { final List> updateActions = buildEnumValuesUpdateActions( @@ -162,6 +160,25 @@ public void buildPlainEnumUpdateActions_WithDifferent_ShouldBuildChangeEnumValue ); } + @Test + public void buildPlainEnumUpdateActions_WithMixedCase_ShouldBuildChangeEnumValueOrderAction() { + final String attributeDefinitionName = "attribute_definition_name_1"; + final List> updateActions = buildEnumValuesUpdateActions( + attributeDefinitionName, + ENUM_VALUES_BAC, + ENUM_VALUES_AB_WITH_DIFFERENT_LABEL + ); + + assertThat(updateActions).containsExactly( + RemoveEnumValues.of(attributeDefinitionName, ENUM_VALUE_C.getKey()), + ChangePlainEnumValueLabel.of(attributeDefinitionName, ENUM_VALUE_A_DIFFERENT_LABEL), + ChangeEnumValueOrder.of(attributeDefinitionName, asList( + ENUM_VALUE_A_DIFFERENT_LABEL, + ENUM_VALUE_B + )) + ); + } + @Test public void buildPlainEnumUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAndRemoveActions() { final List> updateActions = buildEnumValuesUpdateActions( @@ -249,7 +266,6 @@ public void buildPlainEnumUpdateActions_WithDifferentLabels_ShouldReturnChangeLa ); } - @Test public void buildPlainEnumUpdateActions_WithSameLabels_ShouldNotReturnChangeLabelAction() { final List> updateActions = buildEnumValueUpdateActions( @@ -262,6 +278,4 @@ public void buildPlainEnumUpdateActions_WithSameLabels_ShouldNotReturnChangeLabe ChangePlainEnumValueLabel.of("attribute_definition_name_1", ENUM_VALUE_A) ); } - - } From b7d4e54fc33e57a403fdc88650f4631cc8aac8ef Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:48:43 +0100 Subject: [PATCH 50/54] #300: Move fixtures into fixtures class. --- ...BuildFieldDefinitionUpdateActionsTest.java | 54 ++---------- .../types/utils/FieldDefinitionFixtures.java | 84 ++++++++++++++----- .../FieldDefinitionUpdateActionUtilsTest.java | 2 +- 3 files changed, 71 insertions(+), 69 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java index 1a72f9c408..bed9af119c 100644 --- a/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java @@ -23,8 +23,7 @@ import java.util.ArrayList; import java.util.List; -import static com.commercetools.sync.types.FieldDefinitionTestHelper.localizedStringFieldDefinition; -import static com.commercetools.sync.types.FieldDefinitionTestHelper.stringFieldDefinition; +import static com.commercetools.sync.types.utils.FieldDefinitionFixtures.*; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildFieldDefinitionsUpdateActions; import static io.sphere.sdk.json.SphereJsonUtils.readObjectFromResource; import static java.util.Arrays.asList; @@ -36,53 +35,6 @@ import static org.mockito.Mockito.when; public class BuildFieldDefinitionUpdateActionsTest { - private static final String RES_ROOT = - "com/commercetools/sync/types/utils/updatefielddefinitions/fields/"; - - private static final String TYPE_WITH_FIELDS_AB = - RES_ROOT + "type-with-field-definitions-ab.json"; - private static final String TYPE_WITH_FIELDS_ABB = - RES_ROOT + "type-with-field-definitions-abb.json"; - private static final String TYPE_WITH_FIELDS_ABC = - RES_ROOT + "type-with-field-definitions-abc.json"; - private static final String TYPE_WITH_FIELDS_ABCD = - RES_ROOT + "type-with-field-definitions-abcd.json"; - private static final String TYPE_WITH_FIELDS_ABD = - RES_ROOT + "type-with-field-definitions-abd.json"; - private static final String TYPE_WITH_FIELDS_CAB = - RES_ROOT + "type-with-field-definitions-cab.json"; - private static final String TYPE_WITH_FIELDS_CB = - RES_ROOT + "type-with-field-definitions-cb.json"; - private static final String TYPE_WITH_FIELDS_ACBD = - RES_ROOT + "type-with-field-definitions-acbd.json"; - private static final String TYPE_WITH_FIELDS_ADBC = - RES_ROOT + "type-with-field-definitions-adbc.json"; - private static final String TYPE_WITH_FIELDS_CBD = - RES_ROOT + "type-with-field-definitions-cbd.json"; - private static final String TYPE_WITH_FIELDS_ABC_WITH_DIFFERENT_TYPE = - RES_ROOT + "type-with-field-definitions-abc-with-different-type.json"; - - private static final TypeSyncOptions SYNC_OPTIONS = TypeSyncOptionsBuilder - .of(mock(SphereClient.class)) - .build(); - - private static final String FIELD_A = "a"; - private static final String FIELD_B = "b"; - private static final String FIELD_C = "c"; - private static final String FIELD_D = "d"; - private static final String LABEL_EN = "label_en"; - - private static final FieldDefinition FIELD_DEFINITION_A = - stringFieldDefinition(FIELD_A, LABEL_EN, false, TextInputHint.SINGLE_LINE); - private static final FieldDefinition FIELD_DEFINITION_A_LOCALIZED_TYPE = - localizedStringFieldDefinition(FIELD_A, LABEL_EN, false, TextInputHint.SINGLE_LINE); - private static final FieldDefinition FIELD_DEFINITION_B = - stringFieldDefinition(FIELD_B, LABEL_EN, false, TextInputHint.SINGLE_LINE); - private static final FieldDefinition FIELD_DEFINITION_C = - stringFieldDefinition(FIELD_C, LABEL_EN, false, TextInputHint.SINGLE_LINE); - private static final FieldDefinition FIELD_DEFINITION_D = - stringFieldDefinition(FIELD_D, LABEL_EN, false, TextInputHint.SINGLE_LINE); - private static final String TYPE_KEY = "key"; private static final LocalizedString TYPE_NAME = LocalizedString.ofEnglish("name"); private static final LocalizedString TYPE_DESCRIPTION = LocalizedString.ofEnglish("description"); @@ -94,6 +46,10 @@ public class BuildFieldDefinitionUpdateActionsTest { .description(TYPE_DESCRIPTION) .build(); + private static final TypeSyncOptions SYNC_OPTIONS = TypeSyncOptionsBuilder + .of(mock(SphereClient.class)) + .build(); + @Test public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndExistingFieldDefinitions_ShouldBuild3RemoveActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); diff --git a/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java b/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java index da3189bd6b..6644124a47 100644 --- a/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java +++ b/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionFixtures.java @@ -1,4 +1,4 @@ -package com.commercetools.sync.types; +package com.commercetools.sync.types.utils; import io.sphere.sdk.models.LocalizedString; import io.sphere.sdk.models.TextInputHint; @@ -6,30 +6,76 @@ import io.sphere.sdk.types.LocalizedStringFieldType; import io.sphere.sdk.types.StringFieldType; -public final class FieldDefinitionTestHelper { +import javax.annotation.Nonnull; +import javax.annotation.Nullable; - public static FieldDefinition stringFieldDefinition(final String fieldName, - final String labelEng, - boolean required, - final TextInputHint hint) { +public final class FieldDefinitionFixtures { + private static final String RES_ROOT = + "com/commercetools/sync/types/utils/updatefielddefinitions/fields/"; + + static final String TYPE_WITH_FIELDS_AB = + RES_ROOT + "type-with-field-definitions-ab.json"; + static final String TYPE_WITH_FIELDS_ABB = + RES_ROOT + "type-with-field-definitions-abb.json"; + static final String TYPE_WITH_FIELDS_ABC = + RES_ROOT + "type-with-field-definitions-abc.json"; + static final String TYPE_WITH_FIELDS_ABCD = + RES_ROOT + "type-with-field-definitions-abcd.json"; + static final String TYPE_WITH_FIELDS_ABD = + RES_ROOT + "type-with-field-definitions-abd.json"; + static final String TYPE_WITH_FIELDS_CAB = + RES_ROOT + "type-with-field-definitions-cab.json"; + static final String TYPE_WITH_FIELDS_CB = + RES_ROOT + "type-with-field-definitions-cb.json"; + static final String TYPE_WITH_FIELDS_ACBD = + RES_ROOT + "type-with-field-definitions-acbd.json"; + static final String TYPE_WITH_FIELDS_ADBC = + RES_ROOT + "type-with-field-definitions-adbc.json"; + static final String TYPE_WITH_FIELDS_CBD = + RES_ROOT + "type-with-field-definitions-cbd.json"; + static final String TYPE_WITH_FIELDS_ABC_WITH_DIFFERENT_TYPE = + RES_ROOT + "type-with-field-definitions-abc-with-different-type.json"; + + static final String FIELD_A = "a"; + static final String FIELD_B = "b"; + static final String FIELD_C = "c"; + static final String FIELD_D = "d"; + static final String LABEL_EN = "label_en"; + + static final FieldDefinition FIELD_DEFINITION_A = + stringFieldDefinition(FIELD_A, LABEL_EN, false, TextInputHint.SINGLE_LINE); + static final FieldDefinition FIELD_DEFINITION_A_LOCALIZED_TYPE = + localizedStringFieldDefinition(FIELD_A, LABEL_EN, false, TextInputHint.SINGLE_LINE); + static final FieldDefinition FIELD_DEFINITION_B = + stringFieldDefinition(FIELD_B, LABEL_EN, false, TextInputHint.SINGLE_LINE); + static final FieldDefinition FIELD_DEFINITION_C = + stringFieldDefinition(FIELD_C, LABEL_EN, false, TextInputHint.SINGLE_LINE); + static final FieldDefinition FIELD_DEFINITION_D = + stringFieldDefinition(FIELD_D, LABEL_EN, false, TextInputHint.SINGLE_LINE); + + static FieldDefinition stringFieldDefinition(@Nonnull final String fieldName, + @Nonnull final String labelEng, + boolean required, + @Nullable final TextInputHint hint) { return FieldDefinition.of(StringFieldType.of(), - fieldName, - LocalizedString.ofEnglish(labelEng), - required, - hint); + fieldName, + LocalizedString.ofEnglish(labelEng), + required, + hint); } - public static FieldDefinition localizedStringFieldDefinition(final String fieldName, - final String labelEng, - boolean required, - final TextInputHint hint) { + + private static FieldDefinition localizedStringFieldDefinition(@Nonnull final String fieldName, + @Nonnull final String labelEng, + boolean required, + @Nullable final TextInputHint hint) { return FieldDefinition.of(LocalizedStringFieldType.of(), - fieldName, - LocalizedString.ofEnglish(labelEng), - required, - hint); + fieldName, + LocalizedString.ofEnglish(labelEng), + required, + hint); } - private FieldDefinitionTestHelper() { + private FieldDefinitionFixtures() { } } diff --git a/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtilsTest.java b/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtilsTest.java index 584b679b8e..da7bff7e18 100644 --- a/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtilsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtilsTest.java @@ -18,7 +18,7 @@ import java.util.List; import java.util.Optional; -import static com.commercetools.sync.types.FieldDefinitionTestHelper.stringFieldDefinition; +import static com.commercetools.sync.types.utils.FieldDefinitionFixtures.stringFieldDefinition; import static com.commercetools.sync.types.utils.FieldDefinitionUpdateActionUtils.buildActions; import static com.commercetools.sync.types.utils.FieldDefinitionUpdateActionUtils.buildChangeLabelUpdateAction; import static io.sphere.sdk.models.LocalizedString.ofEnglish; From 767825f32c6af27692703a1660365b43b6a7d135 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 17:48:56 +0100 Subject: [PATCH 51/54] #300: Remove unneeded field definitions from test. --- .../types/utils/TypeUpdateActionUtilsTest.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java b/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java index 63a24d6bb6..62cb5d4b66 100644 --- a/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java @@ -2,7 +2,6 @@ import io.sphere.sdk.commands.UpdateAction; import io.sphere.sdk.models.LocalizedString; -import io.sphere.sdk.types.FieldDefinition; import io.sphere.sdk.types.ResourceTypeIdsSetBuilder; import io.sphere.sdk.types.Type; import io.sphere.sdk.types.TypeDraft; @@ -12,14 +11,9 @@ import org.junit.BeforeClass; import org.junit.Test; -import java.util.Arrays; -import java.util.List; import java.util.Optional; import java.util.Set; -import static com.commercetools.sync.types.FieldDefinitionTestHelper.imageUrlFieldDefinition; -import static com.commercetools.sync.types.FieldDefinitionTestHelper.relatedCategoriesFieldDefinition; -import static com.commercetools.sync.types.FieldDefinitionTestHelper.stateFieldDefinition; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildChangeNameUpdateAction; import static com.commercetools.sync.types.utils.TypeUpdateActionUtils.buildSetDescriptionUpdateAction; import static org.assertj.core.api.Assertions.assertThat; @@ -44,21 +38,13 @@ public static void setup() { .addCategories() .build(); - final List fieldDefinitions = - Arrays.asList(stateFieldDefinition(), - imageUrlFieldDefinition(), - relatedCategoriesFieldDefinition()); - - old = mock(Type.class); when(old.getKey()).thenReturn(key); when(old.getName()).thenReturn(name); when(old.getDescription()).thenReturn(desc); - when(old.getFieldDefinitions()).thenReturn(fieldDefinitions); newSame = TypeDraftBuilder.of(key, name, resourceTypeIds) .description(desc) - .fieldDefinitions(fieldDefinitions) .build(); @@ -66,7 +52,6 @@ public static void setup() { LocalizedString.ofEnglish("type for standard categories 2"), resourceTypeIds) .description(LocalizedString.ofEnglish("description for category custom type 2")) - .fieldDefinitions(fieldDefinitions) .build(); } From 98abc39c99ae8432c99813af559a59dc84c97ab0 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 18:26:37 +0100 Subject: [PATCH 52/54] #300: Reword notes. --- docs/usage/PRODUCT_TYPE_SYNC.md | 7 +++++-- docs/usage/TYPE_SYNC.md | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/usage/PRODUCT_TYPE_SYNC.md b/docs/usage/PRODUCT_TYPE_SYNC.md index 3f0c69bdda..b0c03d6c34 100644 --- a/docs/usage/PRODUCT_TYPE_SYNC.md +++ b/docs/usage/PRODUCT_TYPE_SYNC.md @@ -68,8 +68,11 @@ __Note__ The statistics object contains the processing time of the last batch on 2. It is not known by the sync which batch is going to be the last one supplied. #### Important to Note -1. If two matching `attributeDefinition`s on the matching `productType`s have a different `AttributeType`, the sync will -**remove** the existing `attributeDefinition` and then **add** a new `attributeDefinition` with the new `AttributeType`. + +1. If two matching `attributeDefinition`s (old and new) on the matching `productType`s (old and new) have a different `AttributeType`, the sync will +**remove** the existing `attributeDefinition` and then **add** a new `attributeDefinition` with the new `AttributeType`. + +2. The `attributeDefinition` for which the `AttributeType` is not defined (`null`) will not be synced. #### More examples of how to use the sync diff --git a/docs/usage/TYPE_SYNC.md b/docs/usage/TYPE_SYNC.md index 6574e8881e..2b572c2a3b 100644 --- a/docs/usage/TYPE_SYNC.md +++ b/docs/usage/TYPE_SYNC.md @@ -65,8 +65,10 @@ __Note__ The statistics object contains the processing time of the last batch on 2. It is not known by the sync which batch is going to be the last one supplied. #### Important to Note -1. If two matching `fieldDefinition`s on the matching `type`s have a different `FieldType`, the sync will +1. If two matching `fieldDefinition`s (old and new) on the matching `type`s (old and new) have a different `FieldType`, the sync will **remove** the existing `fieldDefinition` and then **add** a new `fieldDefinition` with the new `FieldType`. + +2. The `fieldDefinition` for which the `fieldType` is not defined (`null`) will not be synced. #### More examples of how to use the sync From 7d205287d6f5a7d07794f23a9f16ec2cbcbbbe03 Mon Sep 17 00:00:00 2001 From: aoz Date: Thu, 6 Dec 2018 18:31:20 +0100 Subject: [PATCH 53/54] #300 fix checksStyles --- .../FieldDefinitionsUpdateActionUtils.java | 4 +-- ...BuildFieldDefinitionUpdateActionsTest.java | 28 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java index 5146813cd6..2f352c539b 100644 --- a/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/FieldDefinitionsUpdateActionUtils.java @@ -139,8 +139,8 @@ private static List> buildRemoveFieldDefinitionOrFieldDefinit final Map newFieldDefinitionsNameMap = newFieldDefinitions .stream().collect( - toMap(FieldDefinition::getName, fieldDefinition -> fieldDefinition, - (fieldDefinitionA, fieldDefinitionB) -> { + toMap(FieldDefinition::getName, fieldDefinition -> fieldDefinition, + (fieldDefinitionA, fieldDefinitionB) -> { throw new DuplicateNameException(format("Field definitions have duplicated names. " + "Duplicated field definition name: '%s'. Field definitions names are " + "expected to be unique inside their type.", fieldDefinitionA.getName())); diff --git a/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java b/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java index bed9af119c..17e8e8f92c 100644 --- a/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java +++ b/src/test/java/com/commercetools/sync/types/utils/BuildFieldDefinitionUpdateActionsTest.java @@ -51,7 +51,7 @@ public class BuildFieldDefinitionUpdateActionsTest { .build(); @Test - public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndExistingFieldDefinitions_ShouldBuild3RemoveActions() { + public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefAndExistingFieldDefs_ShouldBuild3RemoveActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final List> updateActions = buildFieldDefinitionsUpdateActions( @@ -68,7 +68,7 @@ public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndExi } @Test - public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndNoOldFieldDefinitions_ShouldNotBuildActions() { + public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefsAndNoOldFieldDefs_ShouldNotBuildActions() { final Type oldType = mock(Type.class); when(oldType.getFieldDefinitions()).thenReturn(emptyList()); when(oldType.getKey()).thenReturn("type_key_1"); @@ -83,7 +83,7 @@ public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinitionsAndNoO } @Test - public void buildFieldDefinitionsUpdateActions_WithNewFieldDefinitionsAndNoOldFieldDefinitions_ShouldBuild3AddActions() { + public void buildFieldDefinitionsUpdateActions_WithNewFieldDefsAndNoOldFieldDefinitions_ShouldBuild3AddActions() { final Type oldType = mock(Type.class); when(oldType.getFieldDefinitions()).thenReturn(emptyList()); when(oldType.getKey()).thenReturn("type_key_1"); @@ -125,7 +125,7 @@ public void buildFieldDefinitionsUpdateActions_WithIdenticalFieldDefinitions_Sho } @Test - public void buildFieldDefinitionsUpdateActions_WithDuplicateFieldDefinitionNames_ShouldNotBuildActionsAndTriggerErrorCb() { + public void buildFieldDefinitionsUpdateActions_WithDuplicateFieldDefNames_ShouldNotBuildActionsAndTriggerErrorCb() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -163,7 +163,7 @@ public void buildFieldDefinitionsUpdateActions_WithDuplicateFieldDefinitionNames } @Test - public void buildFieldDefinitionsUpdateActions_WithOneMissingFieldDefinition_ShouldBuildRemoveFieldDefinitionAction() { + public void buildFieldDefinitionsUpdateActions_WithOneMissingFieldDefinition_ShouldBuildRemoveFieldDefAction() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -181,7 +181,7 @@ public void buildFieldDefinitionsUpdateActions_WithOneMissingFieldDefinition_Sho } @Test - public void buildFieldDefinitionsUpdateActions_WithOneExtraFieldDefinition_ShouldBuildAddFieldDefinitionAction() { + public void buildFieldDefinitionsUpdateActions_WithOneExtraFieldDef_ShouldBuildAddFieldDefinitionAction() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -201,7 +201,7 @@ public void buildFieldDefinitionsUpdateActions_WithOneExtraFieldDefinition_Shoul } @Test - public void buildFieldDefinitionsUpdateActions_WithOneFieldDefinitionSwitch_ShouldBuildRemoveAndAddFieldDefinitionsActions() { + public void buildFieldDefinitionsUpdateActions_WithOneFieldDefSwitch_ShouldBuildRemoveAndAddFieldDefActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -248,7 +248,7 @@ public void buildFieldDefinitionsUpdateActions_WithDifferent_ShouldBuildChangeFi } @Test - public void buildFieldDefinitionsUpdateActions_WithRemovedAndDifferentOrder_ShouldBuildChangeOrderAndRemoveActions() { + public void buildFieldDefinitionsUpdateActions_WithRemovedAndDiffOrder_ShouldBuildChangeOrderAndRemoveActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -302,7 +302,7 @@ public void buildFieldDefinitionsUpdateActions_WithAddedAndDifferentOrder_Should } @Test - public void buildFieldDefinitionsUpdateActions_WithAddedFieldDefinitionInBetween_ShouldBuildChangeOrderAndAddActions() { + public void buildFieldDefinitionsUpdateActions_WithAddedFieldDefInBetween_ShouldBuildChangeOrderAndAddActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -330,7 +330,7 @@ public void buildFieldDefinitionsUpdateActions_WithAddedFieldDefinitionInBetween } @Test - public void buildFieldDefinitionsUpdateActions_WithAddedRemovedAndDifOrder_ShouldBuildAllThreeMoveFieldDefinitionActions() { + public void buildFieldDefinitionsUpdateActions_WithMixedFields_ShouldBuildAllThreeMoveFieldDefActions() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -357,7 +357,7 @@ public void buildFieldDefinitionsUpdateActions_WithAddedRemovedAndDifOrder_Shoul } @Test - public void buildFieldDefinitionsUpdateActions_WithDifferentType_ShouldRemoveOldFieldDefinitionAndAddNewFieldDefinition() { + public void buildFieldDefinitionsUpdateActions_WithDifferentType_ShouldRemoveOldFieldDefAndAddNewFieldDef() { final Type oldType = readObjectFromResource(TYPE_WITH_FIELDS_ABC, Type.class); final TypeDraft newTypeDraft = readObjectFromResource( @@ -377,7 +377,7 @@ public void buildFieldDefinitionsUpdateActions_WithDifferentType_ShouldRemoveOld } @Test - public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinition_ShouldSkipNullFieldDefinitions() { + public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDef_ShouldSkipNullFieldDefs() { // preparation final Type oldType = mock(Type.class); final FieldDefinition oldFieldDefinition = FieldDefinition.of( @@ -415,7 +415,7 @@ public void buildFieldDefinitionsUpdateActions_WithNullNewFieldDefinition_Should } @Test - public void buildFieldDefinitionsUpdateActions_WithDefinitionWithNullName_ShouldBuildChangeFieldDefinitionOrderAction() { + public void buildFieldDefinitionsUpdateActions_WithDefWithNullName_ShouldBuildChangeFieldDefOrderAction() { // preparation final Type oldType = mock(Type.class); final FieldDefinition oldFieldDefinition = FieldDefinition.of( @@ -454,7 +454,7 @@ public void buildFieldDefinitionsUpdateActions_WithDefinitionWithNullName_Should } @Test - public void buildFieldDefinitionsUpdateActions_WithDefinitionWithNullType_ShouldBuildChangeFieldDefinitionOrderAction() { + public void buildFieldDefinitionsUpdateActions_WithDefWithNullType_ShouldBuildChangeFieldDefOrderAction() { // preparation final Type oldType = mock(Type.class); final FieldDefinition oldFieldDefinition = FieldDefinition.of( From edf0981635928980f65788aa52d5701d574ec507 Mon Sep 17 00:00:00 2001 From: Hesham Massoud Date: Thu, 6 Dec 2018 18:37:11 +0100 Subject: [PATCH 54/54] #300: Fix Checkstyle issues. --- .../com/commercetools/sync/types/utils/TypeSyncUtils.java | 6 +++--- .../sync/types/utils/TypeUpdateActionUtils.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java b/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java index e5a9361a85..c55fd32300 100644 --- a/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/TypeSyncUtils.java @@ -23,14 +23,14 @@ public final class TypeSyncUtils { * result. If no update actions are needed, for example in case where both the {@link Type} and the * {@link TypeDraft} have the same fields, an empty {@link List} is returned. * - *

    - * Note: Currently this util doesn't support the following: + * + *

    Note: Currently this util doesn't support the following: *

      *
    • updating the inputHint of a FieldDefinition
      • *
    • removing the EnumValue/LocalizedEnumValue of a FieldDefinition
      • *
    • updating the label of a EnumValue/LocalizedEnumValue of a FieldDefinition
      • *
    - *

    + * * * @param oldType the {@link Type} which should be updated. * @param newType the {@link TypeDraft} where we get the new data. diff --git a/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java b/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java index ad64083f75..7a512fd8c9 100644 --- a/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java +++ b/src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java @@ -63,15 +63,15 @@ public static Optional> buildSetDescriptionUpdateAction( * a list of field definitions in which a duplicate name exists, the error callback is triggered and an empty list * is returned. * - *

    - * Note: Currently this util doesn't support the following: + * + *

    Note: Currently this util doesn't support the following: *

      *
    • updating the inputHint of a FieldDefinition
      • *
    • removing the EnumValue/LocalizedEnumValue of a FieldDefinition
      • *
    • updating the label of a EnumValue/LocalizedEnumValue of a FieldDefinition
      • *
    * TODO: Check GITHUB ISSUE#339 for missing FieldDefinition update actions. - *

    + * * * @param oldType the type which should be updated. * @param newType the type draft where we get the key.