Skip to content

Commit 1653aa0

Browse files
author
salander85
committed
Validate referenced json-values in batch validation and transformation
1 parent 0f91d9a commit 1653aa0

6 files changed

Lines changed: 67 additions & 19 deletions

File tree

‎src/integration-test/java/com/commercetools/sync/integration/services/impl/ProductServiceImplIT.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,8 @@ void cacheKeysToIds_WithAlreadyCachedKeys_ShouldNotMakeRequestsAndReturnCurrentC
259259
spyProductService.cacheKeysToIds(singleton(product.getKey())).toCompletableFuture().join();
260260
assertThat(cache).hasSize(2);
261261
assertThat(cache).containsKeys(product.getKey(), product2.getKey());
262+
cache = spyProductService.cacheKeysToIds(singleton(null)).toCompletableFuture().join();
263+
assertThat(cache).hasSize(2);
262264

263265
// verify only 1 request was made to fetch id the first time, but not second time since it's
264266
// already in cache.

‎src/main/java/com/commercetools/sync/commons/utils/CustomValueConverter.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
import com.commercetools.api.models.type.CustomFieldsDraft;
55
import com.fasterxml.jackson.databind.JsonNode;
66
import com.fasterxml.jackson.databind.ObjectMapper;
7+
import com.fasterxml.jackson.databind.node.JsonNodeType;
78
import io.vrap.rmf.base.client.utils.json.JsonUtils;
89
import java.util.Objects;
910
import javax.annotation.Nullable;
11+
import org.apache.commons.lang3.StringUtils;
1012

1113
public final class CustomValueConverter {
1214

@@ -27,4 +29,17 @@ public static JsonNode convertCustomValueObjDataToJsonNode(@Nullable final Objec
2729
final JsonNode jsonNode = objectMapper.convertValue(data, JsonNode.class);
2830
return jsonNode;
2931
}
32+
33+
/**
34+
* Takes a value of type JsonNode and checks if it's a valid string value.
35+
*
36+
* @param node - a jsonNode which might contain text
37+
* @return true if the given node is not null, not blank and does not contain a "null" value.
38+
*/
39+
public static boolean isValidTextNode(@Nullable JsonNode node) {
40+
return node != null
41+
&& JsonNodeType.STRING.equals(node.getNodeType())
42+
&& !StringUtils.isBlank(node.asText())
43+
&& !"null".equals(node.asText());
44+
}
3045
}

‎src/main/java/com/commercetools/sync/products/helpers/ProductBatchValidator.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.commercetools.sync.products.helpers;
22

3+
import static com.commercetools.sync.commons.utils.CustomValueConverter.isValidTextNode;
34
import static com.commercetools.sync.commons.utils.ResourceIdentifierUtils.REFERENCE_ID_FIELD;
45
import static com.commercetools.sync.commons.utils.ResourceIdentifierUtils.isReferenceOfType;
56
import static java.lang.String.format;
@@ -285,7 +286,9 @@ private static Set<String> getReferencedKeysWithReferenceTypeId(
285286

286287
return allAttributeReferences.stream()
287288
.filter(reference -> isReferenceOfType(reference, referenceTypeId))
288-
.map(reference -> reference.get(REFERENCE_ID_FIELD).asText())
289+
.map(reference -> reference.get(REFERENCE_ID_FIELD))
290+
.filter(field -> isValidTextNode(field))
291+
.map(field -> field.asText())
289292
.filter(Objects::nonNull)
290293
.collect(Collectors.toSet());
291294
}

‎src/main/java/com/commercetools/sync/services/impl/BaseTransformServiceImpl.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.commercetools.sync.services.impl;
22

3+
import static com.commercetools.sync.commons.utils.CustomValueConverter.isValidTextNode;
34
import static com.commercetools.sync.commons.utils.ResourceIdentifierUtils.REFERENCE_ID_FIELD;
45
import static com.commercetools.sync.commons.utils.ResourceIdentifierUtils.REFERENCE_TYPE_ID_FIELD;
56
import static java.lang.String.format;
@@ -34,7 +35,6 @@
3435
import java.util.stream.Collectors;
3536
import javax.annotation.Nonnull;
3637
import javax.annotation.Nullable;
37-
import org.apache.commons.lang3.StringUtils;
3838
import org.apache.commons.text.StringEscapeUtils;
3939

4040
public abstract class BaseTransformServiceImpl {
@@ -224,12 +224,9 @@ protected void cacheResourceReferenceKeys(
224224
}
225225

226226
private void fillReferenceIdToKeyCache(@Nullable JsonNode id, @Nullable JsonNode key) {
227-
if (id != null && !StringUtils.isBlank(id.asText())) {
227+
if (isValidTextNode(id)) {
228228
final String idValue = id.asText();
229-
final String keyValue =
230-
key != null && !StringUtils.isBlank(key.asText())
231-
? key.asText()
232-
: KEY_IS_NOT_SET_PLACE_HOLDER;
229+
final String keyValue = isValidTextNode(key) ? key.asText() : KEY_IS_NOT_SET_PLACE_HOLDER;
233230
referenceIdToKeyCache.add(idValue, keyValue);
234231
}
235232
}

‎src/test/java/com/commercetools/sync/products/helpers/ProductBatchValidatorTest.java‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,18 @@
1616
import com.commercetools.api.models.common.PriceDraftBuilder;
1717
import com.commercetools.api.models.custom_object.CustomObjectReference;
1818
import com.commercetools.api.models.customer_group.CustomerGroupResourceIdentifierBuilder;
19-
import com.commercetools.api.models.product.Attribute;
20-
import com.commercetools.api.models.product.AttributeBuilder;
21-
import com.commercetools.api.models.product.ProductDraft;
22-
import com.commercetools.api.models.product.ProductVariantDraft;
23-
import com.commercetools.api.models.product.ProductVariantDraftBuilder;
19+
import com.commercetools.api.models.product.*;
2420
import com.commercetools.api.models.state.StateResourceIdentifierBuilder;
2521
import com.commercetools.api.models.tax_category.TaxCategoryResourceIdentifierBuilder;
2622
import com.commercetools.api.models.type.CustomFieldsDraftBuilder;
2723
import com.commercetools.api.models.type.FieldContainerBuilder;
2824
import com.commercetools.api.models.type.TypeResourceIdentifierBuilder;
25+
import com.commercetools.sync.commons.utils.ResourceIdentifierUtils;
2926
import com.commercetools.sync.customobjects.helpers.CustomObjectCompositeIdentifier;
3027
import com.commercetools.sync.products.ProductSyncMockUtils;
3128
import com.commercetools.sync.products.ProductSyncOptions;
3229
import com.commercetools.sync.products.ProductSyncOptionsBuilder;
3330
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
34-
import com.fasterxml.jackson.databind.node.NullNode;
3531
import com.fasterxml.jackson.databind.node.ObjectNode;
3632
import java.math.BigDecimal;
3733
import java.util.ArrayList;
@@ -142,10 +138,29 @@ void getReferencedProductKeys_WithProductRefAsValue_ShouldReturnKeyInSet() {
142138
}
143139

144140
@Test
145-
void getProductKeyFromReference_WithNullJsonNode_ShouldReturnEmptyOpt() {
146-
final NullNode nullNode = JsonNodeFactory.instance.nullNode();
141+
void getProductKeyFromReference_WithNullId_ShouldReturnEmptyOpt() {
142+
final ObjectNode referenceValue = JsonNodeFactory.instance.objectNode();
143+
referenceValue.put(ResourceIdentifierUtils.REFERENCE_TYPE_ID_FIELD, ProductReference.PRODUCT);
144+
referenceValue.set(
145+
ResourceIdentifierUtils.REFERENCE_ID_FIELD, JsonNodeFactory.instance.nullNode());
147146
final Attribute productReferenceAttribute =
148-
AttributeBuilder.of().name("foo").value(nullNode).build();
147+
AttributeBuilder.of().name("foo").value(referenceValue).build();
148+
149+
final ProductVariantDraft productVariantDraft =
150+
ProductVariantDraftBuilder.of().attributes(productReferenceAttribute).build();
151+
152+
final Set<String> result = ProductBatchValidator.getReferencedProductKeys(productVariantDraft);
153+
154+
assertThat(result).isEmpty();
155+
}
156+
157+
@Test
158+
void getProductKeyFromReference_WithNullStringIdValue_ShouldReturnEmptyOpt() {
159+
final Attribute productReferenceAttribute =
160+
AttributeBuilder.of()
161+
.name("foo")
162+
.value(ProductSyncMockUtils.getProductReferenceWithId("null"))
163+
.build();
149164

150165
final ProductVariantDraft productVariantDraft =
151166
ProductVariantDraftBuilder.of().attributes(productReferenceAttribute).build();

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,13 +398,26 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
398398
final ApiHttpResponse<GraphQLResponse> productTypesResponse =
399399
TestUtils.mockGraphQLResponse(jsonStringProductTypes);
400400

401+
String jsonStringTaxCategory =
402+
"{ \"taxCategories\": {\"results\":[{\"id\":\"ebbe95fb-2282-4f9a-8747-fbe440e02dc0\","
403+
+ "\"key\":\"null\"}]}}";
404+
final ApiHttpResponse<GraphQLResponse> taxCategoryResponse =
405+
TestUtils.mockGraphQLResponse(jsonStringTaxCategory);
406+
407+
String jsonStringState =
408+
"{ \"states\": {\"results\":[{\"id\": " + null + "," + "\"key\": " + null + "}]}}";
409+
final ApiHttpResponse<GraphQLResponse> stateResponse =
410+
TestUtils.mockGraphQLResponse(jsonStringState);
411+
401412
final ByProjectKeyGraphqlPost byProjectKeyGraphQlPost = mock(ByProjectKeyGraphqlPost.class);
402413

403414
when(sourceClient.graphql()).thenReturn(mock());
404415
when(sourceClient.graphql().post(any(GraphQLRequest.class)))
405416
.thenReturn(byProjectKeyGraphQlPost);
406417
when(byProjectKeyGraphQlPost.execute())
407418
.thenReturn(CompletableFuture.completedFuture(productTypesResponse))
419+
.thenReturn(CompletableFuture.completedFuture(taxCategoryResponse))
420+
.thenReturn(CompletableFuture.completedFuture(stateResponse))
408421
.thenReturn(CompletableFuture.completedFuture(mock(ApiHttpResponse.class)));
409422

410423
mockAttributeCustomObjectReference(sourceClient);
@@ -424,9 +437,12 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
424437

425438
assertThat(productKey1)
426439
.hasValueSatisfying(
427-
productDraft ->
428-
assertThat(productDraft.getProductType().getKey())
429-
.isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER));
440+
productDraft -> {
441+
assertThat(productDraft.getProductType().getKey())
442+
.isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
443+
assertThat(productDraft.getTaxCategory().getKey())
444+
.isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
445+
});
430446
}
431447

432448
@Test

0 commit comments

Comments
 (0)