Avoid building resourceIdentifiers with invalid key values

Author: salander85

src/main/java/com/commercetools/sync/commons/utils/SyncUtils.java

@@ -2,6 +2,7 @@
 
 import com.commercetools.api.models.common.Reference;
 import com.commercetools.api.models.common.ResourceIdentifier;
+import com.commercetools.sync.services.impl.BaseTransformServiceImpl;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -70,7 +71,8 @@ ResourceIdentifierT getResourceIdentifierWithKey(
           @Nullable final String key,
           final BiFunction<String, String, ResourceIdentifierT> toResourceIdentifier) {
 
-    if (!StringUtils.isEmpty(key)) {
+    if (!StringUtils.isBlank(key)
+        && !BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER.equals(key)) {
       return toResourceIdentifier.apply(null, key);
     }
     return toResourceIdentifier.apply(id, null);

src/test/java/com/commercetools/sync/commons/utils/SyncUtilsTest.java

@@ -1,5 +1,6 @@
 package com.commercetools.sync.commons.utils;
 
+import static com.commercetools.sync.services.impl.BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER;
 import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -161,4 +162,40 @@ void getResourceIdentifierWithKey_WithNullReference_ShouldReturnNull() {
             (id, key) -> CategoryResourceIdentifierBuilder.of().id(id).key(key).build());
     assertThat(resourceIdentifier).isNull();
   }
+
+  @Test
+  void getResourceIdentifierWithKey_WithNotCachedReference_ShouldReturnResourceIdentifierWithId() {
+    final String categoryId = UUID.randomUUID().toString();
+
+    final CategoryReference categoryReference =
+        CategoryReferenceBuilder.of().id(categoryId).build();
+
+    final CategoryResourceIdentifier resourceIdentifier =
+        SyncUtils.getResourceIdentifierWithKey(
+            categoryReference,
+            referenceIdToKeyCache,
+            (id, key) -> CategoryResourceIdentifierBuilder.of().id(id).key(key).build());
+
+    assertThat(resourceIdentifier).isNotNull();
+    assertThat(resourceIdentifier.getId()).isEqualTo(categoryId);
+  }
+
+  @Test
+  void
+      getResourceIdentifierWithKey_WithCachedReferenceIsEmptyPlaceholder_ShouldReturnResourceIdentifierWithId() {
+    final String categoryId = UUID.randomUUID().toString();
+    referenceIdToKeyCache.add(categoryId, KEY_IS_NOT_SET_PLACE_HOLDER);
+
+    final CategoryReference categoryReference =
+        CategoryReferenceBuilder.of().id(categoryId).build();
+
+    final CategoryResourceIdentifier resourceIdentifier =
+        SyncUtils.getResourceIdentifierWithKey(
+            categoryReference,
+            referenceIdToKeyCache,
+            (id, key) -> CategoryResourceIdentifierBuilder.of().id(id).key(key).build());
+
+    assertThat(resourceIdentifier).isNotNull();
+    assertThat(resourceIdentifier.getId()).isEqualTo(categoryId);
+  }
 }

src/test/java/com/commercetools/sync/products/utils/ProductTransformUtilsTest.java

@@ -1,5 +1,6 @@
 package com.commercetools.sync.products.utils;
 
+import static com.commercetools.sync.services.impl.BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -25,7 +26,6 @@
 import com.commercetools.sync.commons.utils.ReferenceIdToKeyCache;
 import com.commercetools.sync.commons.utils.TestUtils;
 import com.commercetools.sync.products.ProductSyncMockUtils;
-import com.commercetools.sync.services.impl.BaseTransformServiceImpl;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -439,9 +439,9 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
         .hasValueSatisfying(
             productDraft -> {
               assertThat(productDraft.getProductType().getKey())
-                  .isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
+                  .isEqualTo(KEY_IS_NOT_SET_PLACE_HOLDER);
               assertThat(productDraft.getTaxCategory().getKey())
-                  .isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
+                  .isEqualTo(KEY_IS_NOT_SET_PLACE_HOLDER);
             });
   }
 
@@ -451,9 +451,8 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
           throws Exception {
     // preparation
     final ProjectApiRoot sourceClient = mock(ProjectApiRoot.class);
-    referenceIdToKeyCache.add(
-        "cda0dbf7-b42e-40bf-8453-241d5b587f93",
-        BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
+    referenceIdToKeyCache.add("cda0dbf7-b42e-40bf-8453-241d5b587f93", KEY_IS_NOT_SET_PLACE_HOLDER);
+    referenceIdToKeyCache.add("ebbe95fb-2282-4f9a-8747-fbe440e02dc0", KEY_IS_NOT_SET_PLACE_HOLDER);
     final List<ProductProjection> productPage =
         Arrays.asList(
             ProductSyncMockUtils.createProductFromJson("product-with-unresolved-references.json"));
@@ -482,16 +481,19 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
             .join();
 
     // assertions
-
     final Optional<ProductDraft> productKey1 =
         productsResolved.stream()
             .filter(productDraft -> "productKeyResolved".equals(productDraft.getKey()))
             .findFirst();
 
     assertThat(productKey1)
         .hasValueSatisfying(
-            productDraft ->
-                assertThat(productDraft.getProductType().getKey()).isEqualTo("productTypeKey"));
+            productDraft -> {
+              assertThat(productDraft.getProductType().getKey()).isEqualTo("productTypeKey");
+              assertThat(productDraft.getTaxCategory().getId())
+                  .isEqualTo("ebbe95fb-2282-4f9a-8747-fbe440e02dc0");
+              assertThat(productDraft.getTaxCategory().getKey()).isNull();
+            });
   }
 
   @Test