Skip to content

Commit ee14261

Browse files
lojzatranlojzatran
authored
fix(bug): products with no variants should be synced (#1204)
* fix(bug): products with no variants should be synced * fix(bug): add grgit
1 parent 5e9d162 commit ee14261

File tree

5 files changed

+126
-30
lines changed

5 files changed

+126
-30
lines changed

‎build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
plugins {
22
id 'com.adarshr.test-logger' version '4.0.0'
3+
id 'org.ajoberstar.grgit' version '5.2.2'
34
id "com.github.ben-manes.versions" version '0.51.0'
45
id 'ru.vyarus.mkdocs' version '4.0.1' apply false
56
id "com.github.spotbugs" version "6.0.9"

‎src/integration-test/java/com/commercetools/sync/integration/externalsource/products/ProductSyncIT.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
import java.util.Locale;
7979
import java.util.Optional;
8080
import java.util.Set;
81+
import java.util.UUID;
8182
import javax.annotation.Nonnull;
8283
import org.junit.jupiter.api.AfterAll;
8384
import org.junit.jupiter.api.BeforeAll;
@@ -955,6 +956,65 @@ void sync_withANullDraftInBatch_ShouldNotSyncItAndTriggerErrorCallBack() {
955956
assertThat(warningCallBackMessages).isEmpty();
956957
}
957958

959+
@Test
960+
void sync_shouldSyncProductsWithoutAnyVariants() {
961+
// preparation
962+
final List<ProductUpdateAction> updateActions = new ArrayList<>();
963+
final TriConsumer<SyncException, Optional<ProductDraft>, Optional<ProductProjection>>
964+
warningCallBack =
965+
(exception, newResource, oldResource) ->
966+
warningCallBackMessages.add(exception.getMessage());
967+
968+
final ProductSyncOptions customOptions =
969+
ProductSyncOptionsBuilder.of(TestClientUtils.CTP_TARGET_CLIENT)
970+
.errorCallback(
971+
(exception, oldResource, newResource, actions) ->
972+
collectErrors(exception.getMessage(), exception.getCause()))
973+
.warningCallback(warningCallBack)
974+
.beforeUpdateCallback(
975+
(actions, draft, old) -> {
976+
updateActions.addAll(actions);
977+
return actions;
978+
})
979+
.build();
980+
981+
String productKey1 = UUID.randomUUID().toString();
982+
983+
final ProductDraft productDraft1 =
984+
ProductDraftBuilder.of()
985+
.key(productKey1)
986+
.productType(
987+
ProductTypeResourceIdentifierBuilder.of().key(productType.getKey()).build())
988+
.taxCategory((TaxCategoryResourceIdentifier) null)
989+
.name(LocalizedString.of(Locale.ENGLISH, "name"))
990+
.slug(LocalizedString.of(Locale.ENGLISH, "slug"))
991+
.description(LocalizedString.of(Locale.ENGLISH, "description"))
992+
.categories(emptyList())
993+
.publish(true)
994+
.build();
995+
996+
final ProductDraft productDraft2 =
997+
ProductDraftBuilder.of()
998+
.key(productKey1)
999+
.productType(
1000+
ProductTypeResourceIdentifierBuilder.of().key(productType.getKey()).build())
1001+
.taxCategory((TaxCategoryResourceIdentifier) null)
1002+
.name(LocalizedString.of(Locale.ENGLISH, "name"))
1003+
.slug(LocalizedString.of(Locale.ENGLISH, "slug"))
1004+
.description(LocalizedString.of(Locale.ENGLISH, "description2"))
1005+
.categories(emptyList())
1006+
.publish(true)
1007+
.build();
1008+
1009+
// test
1010+
final ProductSync productSync = new ProductSync(customOptions);
1011+
productSync.sync(singletonList(productDraft1)).toCompletableFuture().join();
1012+
final ProductSyncStatistics syncStatistics =
1013+
productSync.sync(singletonList(productDraft2)).toCompletableFuture().join();
1014+
1015+
assertThat(syncStatistics).hasValues(2, 1, 1, 0, 0);
1016+
}
1017+
9581018
@Test
9591019
void sync_withProductContainingAttributeChanges_shouldSyncProductCorrectly() {
9601020
// preparation

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ public class ProductBatchValidator
4141
"ProductDraft with name: %s doesn't have a key. "
4242
+ "Please make sure all product drafts have keys.";
4343
static final String PRODUCT_DRAFT_IS_NULL = "ProductDraft is null.";
44+
45+
static final String PRODUCT_MASTER_VARIANT_DRAFT_IS_NULL =
46+
"MasterVariant is null and Variants are not null for ProductDraft with key '%s'.";
47+
4448
static final String PRODUCT_VARIANT_DRAFT_IS_NULL =
4549
"ProductVariantDraft at position '%d' of ProductDraft " + "with key '%s' is null.";
4650
static final String PRODUCT_VARIANT_DRAFT_SKU_NOT_SET =
@@ -151,7 +155,9 @@ private void collectReferencedKeysInVariants(
151155
@Nonnull
152156
private List<ProductVariantDraft> getAllVariants(@Nonnull final ProductDraft productDraft) {
153157
final List<ProductVariantDraft> allVariants = new ArrayList<>();
154-
allVariants.add(productDraft.getMasterVariant());
158+
if (productDraft.getMasterVariant() != null) {
159+
allVariants.add(productDraft.getMasterVariant());
160+
}
155161
if (productDraft.getVariants() != null) {
156162
allVariants.addAll(
157163
productDraft.getVariants().stream()
@@ -203,10 +209,19 @@ private List<String> getVariantDraftErrorsInAllVariants(
203209
@Nonnull final ProductDraft productDraft) {
204210
final List<String> errorMessages = new ArrayList<>();
205211

212+
final ProductVariantDraft masterVariant = productDraft.getMasterVariant();
213+
final List<ProductVariantDraft> variants = productDraft.getVariants();
214+
215+
if (masterVariant == null) {
216+
if (variants != null && !variants.isEmpty()) {
217+
errorMessages.add(format(PRODUCT_MASTER_VARIANT_DRAFT_IS_NULL, productDraft.getKey()));
218+
}
219+
return errorMessages;
220+
}
221+
206222
// don't filter the nulls
207223
final List<ProductVariantDraft> allVariants = new ArrayList<>();
208-
allVariants.add(productDraft.getMasterVariant());
209-
final List<ProductVariantDraft> variants = productDraft.getVariants();
224+
allVariants.add(masterVariant);
210225
if (variants != null) {
211226
allVariants.addAll(variants);
212227
}

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,14 @@ public CompletionStage<ProductDraft> resolveReferences(@Nonnull final ProductDra
161161
private CompletionStage<ProductDraftBuilder> resolveAllVariantsReferences(
162162
@Nonnull final ProductDraftBuilder draftBuilder) {
163163
final ProductVariantDraft masterVariantDraft = draftBuilder.getMasterVariant();
164-
return variantReferenceResolver
165-
.resolveReferences(masterVariantDraft)
166-
.thenApply(draftBuilder::masterVariant)
167-
.thenCompose(this::resolveVariantsReferences);
164+
if (masterVariantDraft == null) {
165+
return CompletableFuture.completedFuture(draftBuilder);
166+
} else {
167+
return variantReferenceResolver
168+
.resolveReferences(masterVariantDraft)
169+
.thenApply(draftBuilder::masterVariant)
170+
.thenCompose(this::resolveVariantsReferences);
171+
}
168172
}
169173

170174
@Nonnull

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

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

3+
import static java.lang.String.format;
34
import static java.util.Arrays.asList;
45
import static java.util.Collections.singletonList;
56
import static org.apache.commons.lang3.StringUtils.EMPTY;
@@ -241,8 +242,7 @@ void validateAndCollectReferencedKeys_WithEmptyDraft_ShouldHaveEmptyResult() {
241242

242243
assertThat(errorCallBackMessages).hasSize(1);
243244
assertThat(errorCallBackMessages.get(0))
244-
.isEqualTo(
245-
String.format(ProductBatchValidator.PRODUCT_DRAFT_KEY_NOT_SET, productDraft.getName()));
245+
.isEqualTo(format(ProductBatchValidator.PRODUCT_DRAFT_KEY_NOT_SET, productDraft.getName()));
246246
assertThat(validDrafts).isEmpty();
247247
}
248248

@@ -255,13 +255,12 @@ void validateAndCollectReferencedKeys_WithEmptyDraft_ShouldHaveEmptyResult() {
255255

256256
assertThat(errorCallBackMessages).hasSize(1);
257257
assertThat(errorCallBackMessages.get(0))
258-
.isEqualTo(
259-
String.format(ProductBatchValidator.PRODUCT_DRAFT_KEY_NOT_SET, productDraft.getName()));
258+
.isEqualTo(format(ProductBatchValidator.PRODUCT_DRAFT_KEY_NOT_SET, productDraft.getName()));
260259
assertThat(validDrafts).isEmpty();
261260
}
262261

263262
@Test
264-
void validateAndCollectReferencedKeys_WithNullVariant_ShouldHaveValidationErrors() {
263+
void validateAndCollectReferencedKeys_WithNullVariant_ShouldBeValid() {
265264
final String productDraftKey = "key";
266265
final int variantPosition = 0;
267266

@@ -270,14 +269,8 @@ void validateAndCollectReferencedKeys_WithNullVariant_ShouldHaveValidationErrors
270269

271270
final Set<ProductDraft> validDrafts = getValidDrafts(Collections.singletonList(productDraft));
272271

273-
assertThat(validDrafts).isEmpty();
274-
assertThat(errorCallBackMessages).hasSize(1);
275-
assertThat(errorCallBackMessages.get(0))
276-
.isEqualTo(
277-
String.format(
278-
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_IS_NULL,
279-
variantPosition,
280-
productDraftKey));
272+
assertThat(validDrafts).hasSize(1);
273+
assertThat(new ArrayList<>(validDrafts).get(0)).isEqualTo(productDraft);
281274
}
282275

283276
@Test
@@ -296,11 +289,11 @@ void validateAndCollectReferencedKeys_WithVariantButNoKeyAndSku_ShouldHaveValida
296289
assertThat(errorCallBackMessages).hasSize(2);
297290
assertThat(errorCallBackMessages)
298291
.containsExactlyInAnyOrder(
299-
String.format(
292+
format(
300293
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_SKU_NOT_SET,
301294
variantPosition,
302295
productDraftKey),
303-
String.format(
296+
format(
304297
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_KEY_NOT_SET,
305298
variantPosition,
306299
productDraftKey));
@@ -324,7 +317,7 @@ void validateAndCollectReferencedKeys_WithNoVariantKey_ShouldHaveKeyValidationEr
324317
assertThat(errorCallBackMessages).hasSize(1);
325318
assertThat(errorCallBackMessages.get(0))
326319
.isEqualTo(
327-
String.format(
320+
format(
328321
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_KEY_NOT_SET,
329322
variantPosition,
330323
productDraftKey));
@@ -347,7 +340,7 @@ void validateAndCollectReferencedKeys_WithNoVariantSku_ShouldHaveSkuValidationEr
347340
assertThat(errorCallBackMessages).hasSize(1);
348341
assertThat(errorCallBackMessages.get(0))
349342
.isEqualTo(
350-
String.format(
343+
format(
351344
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_SKU_NOT_SET,
352345
variantPosition,
353346
productDraftKey));
@@ -431,21 +424,44 @@ void validateAndCollectReferencedKeys_WithDrafts_ShouldValidateCorrectly() {
431424
assertThat(errorCallBackMessages)
432425
.containsExactlyInAnyOrder(
433426
ProductBatchValidator.PRODUCT_DRAFT_IS_NULL,
434-
String.format(ProductBatchValidator.PRODUCT_DRAFT_KEY_NOT_SET, "null"),
435-
String.format(
436-
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_IS_NULL,
437-
0,
427+
format(ProductBatchValidator.PRODUCT_DRAFT_KEY_NOT_SET, "null"),
428+
format(
429+
ProductBatchValidator.PRODUCT_MASTER_VARIANT_DRAFT_IS_NULL,
438430
inValidProductDraft1.getKey()),
439-
String.format(
431+
format(
440432
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_SKU_NOT_SET,
441433
0,
442434
inValidProductDraft2.getKey()),
443-
String.format(
435+
format(
444436
ProductBatchValidator.PRODUCT_VARIANT_DRAFT_SKU_NOT_SET,
445437
1,
446438
inValidProductDraft2.getKey()));
447439
}
448440

441+
@Test
442+
void
443+
validateAndCollectReferencedKeys_WithNullMasterVariantsAndOneVariant_shouldHaveValidationErrors() {
444+
final ProductDraft productDraft = mock(ProductDraft.class);
445+
when(productDraft.getKey()).thenReturn("key");
446+
447+
final ProductVariantDraft validVariantDraft =
448+
ProductVariantDraftBuilder.of().key("variantKey").sku("variantSku").build();
449+
450+
when(productDraft.getVariants()).thenReturn(singletonList(validVariantDraft));
451+
452+
final ProductBatchValidator productBatchValidator =
453+
new ProductBatchValidator(syncOptions, syncStatistics);
454+
final ImmutablePair<Set<ProductDraft>, ProductBatchValidator.ReferencedKeys> pair =
455+
productBatchValidator.validateAndCollectReferencedKeys(singletonList(productDraft));
456+
457+
assertThat(pair.getLeft()).isEmpty();
458+
assertThat(this.errorCallBackMessages).hasSize(1);
459+
assertThat(this.errorCallBackMessages.get(0))
460+
.isEqualTo(
461+
format(
462+
ProductBatchValidator.PRODUCT_MASTER_VARIANT_DRAFT_IS_NULL, productDraft.getKey()));
463+
}
464+
449465
@Test
450466
void
451467
validateAndCollectReferencedKeys_WithCustomerGroupRefsInPrices_ShouldCollectReferencesCorrectly() {

0 commit comments

Comments
 (0)