Skip to content

Commit ff8a981

Browse files
ahmetozsalander85
ahmetoz
and
salander85
authored
#235 Populate keyToId caches in services before reference resolution to improve the performance (#574)
- 🎉 **New Features** (1) - Introduced new concept for the validation of `the drafts in batches` for each `Sync` instance, exposed with `BaseBatchValidator` implementations (i.e. ProductBatchValidator, CategoryBatchValidator). [#233](#233) - ✨ **Enhancements** (2) - **Category Sync** - Passed category keys in batch to `cacheKeysToIds` method of `CategoryService` to avoid fetching all categories for every batch. [#235](#235) - Populated `keyToId` caches in services before reference resolution to improve the performance of the library with collecting referenced keys in batches of drafts. [#235](#235) Co-authored-by: salander85 <sarah.lander@commercetools.com>
1 parent 629fc47 commit ff8a981

58 files changed

Lines changed: 2991 additions & 1353 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎docs/RELEASE_NOTES.md‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,21 @@
2929
-->
3030

3131
<!--
32-
### 2.x.x - MM xx, 2020
33-
[Commits](https://github.com/commercetools/commercetools-sync-java/compare/2.0.0...2.0.1) |
34-
[Javadoc](https://commercetools.github.io/commercetools-sync-java/v/2.0.1/) |
35-
[Jar](https://bintray.com/commercetools/maven/commercetools-sync-java/2.0.1)
32+
33+
### 2.2.0 - MM xx,, 2020
34+
[Commits](https://github.com/commercetools/commercetools-sync-java/compare/2.1.0...2.2.0) |
35+
[Javadoc](https://commercetools.github.io/commercetools-sync-java/v/2.2.0/) |
36+
[Jar](https://bintray.com/commercetools/maven/commercetools-sync-java/2.2.0)
37+
38+
- 🎉 **New Features** (1)
39+
- Introduced new concept for the validation of `the drafts in batches` for each `Sync` instance, exposed with
40+
`BaseBatchValidator` implementations (i.e. ProductBatchValidator, CategoryBatchValidator). [#233](https://github.com/commercetools/commercetools-sync-java/issues/233)
41+
42+
- ✨ **Enhancements** (2)
43+
- **Category Sync** - Passed category keys in batch to `cacheKeysToIds` method of `CategoryService` to avoid fetching all categories for every batch.
44+
[#235](https://github.com/commercetools/commercetools-sync-java/issues/235)
45+
- Populated `keyToId` caches in services before reference resolution to improve the performance of the library
46+
with collecting referenced keys in batches of drafts. [#235](https://github.com/commercetools/commercetools-sync-java/issues/235)
3647
-->
3748

3849
### 2.1.0 - Sep 21, 2020

‎src/integration-test/java/com/commercetools/sync/integration/ctpprojectsource/categories/CategorySyncIT.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,6 @@ void syncDrafts_fromCategoriesWithoutKeys_ShouldNotUpdateCategories() {
409409
assertThat(fieldErrors).allSatisfy(error -> assertThat(error.getField()).isEqualTo("slug.en"));
410410
});
411411

412-
assertThat(callBackWarningResponses)
413-
.hasSize(2)
414-
.allSatisfy(warningMessage ->
415-
assertThat(warningMessage)
416-
.matches("Category with id: '.*' has no key set. Keys are required for category matching."));
412+
assertThat(callBackWarningResponses).isEmpty();
417413
}
418414
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,6 @@ private SphereClient buildClientWithConcurrentModificationUpdateAndFailedFetchOn
457457

458458
final ProductQuery anyProductQuery = any(ProductQuery.class);
459459
when(spyClient.execute(anyProductQuery))
460-
.thenCallRealMethod() // cache product keys
461460
.thenCallRealMethod() // Call real fetch on fetching matching products
462461
.thenReturn(CompletableFutureUtils.exceptionallyCompletedFuture(new BadGatewayException()));
463462

@@ -509,7 +508,6 @@ private SphereClient buildClientWithConcurrentModificationUpdateAndNotFoundFetch
509508
final ProductQuery anyProductQuery = any(ProductQuery.class);
510509

511510
when(spyClient.execute(anyProductQuery))
512-
.thenCallRealMethod() // cache product keys
513511
.thenCallRealMethod() // Call real fetch on fetching matching products
514512
.thenReturn(CompletableFuture.completedFuture(PagedQueryResult.empty()));
515513

‎src/integration-test/java/com/commercetools/sync/integration/externalsource/states/StateSyncIT.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,6 @@ private SphereClient buildClientWithConcurrentModificationUpdateAndFailedFetchOn
460460

461461
final StateQuery stateQuery = any(StateQuery.class);
462462
when(spyClient.execute(stateQuery))
463-
.thenCallRealMethod() // cache state keys
464463
.thenCallRealMethod() // Call real fetch on fetching matching states
465464
.thenReturn(exceptionallyCompletedFuture(new BadGatewayException()));
466465

@@ -520,7 +519,6 @@ private SphereClient buildClientWithConcurrentModificationUpdateAndNotFoundFetch
520519
final StateQuery stateQuery = any(StateQuery.class);
521520

522521
when(spyClient.execute(stateQuery))
523-
.thenCallRealMethod() // cache state keys
524522
.thenCallRealMethod() // Call real fetch on fetching matching states
525523
.thenReturn(completedFuture(PagedQueryResult.empty()));
526524

@@ -1026,7 +1024,7 @@ void sync_WithFailureInKeysToIdCreation_ShouldAddErrorMessage() {
10261024
assertThat(stateSyncStatistics).hasValues(3, 0, 0, 3, 0);
10271025
Assertions.assertThat(errorCallBackExceptions).isNotEmpty();
10281026
Assertions.assertThat(errorCallBackMessages).isNotEmpty();
1029-
Assertions.assertThat(errorCallBackMessages.get(0)).isEqualTo("Failed to build a cache of state keys to ids.");
1027+
Assertions.assertThat(errorCallBackMessages.get(0)).isEqualTo("Failed to build a cache of keys to ids.");
10301028
Assertions.assertThat(warningCallBackMessages).isEmpty();
10311029
}
10321030

@@ -1058,8 +1056,7 @@ void sync_WithStateWithEmptyTransition_ShouldAddErrorMessage() {
10581056
Assertions.assertThat(errorCallBackExceptions).isNotEmpty();
10591057
Assertions.assertThat(errorCallBackMessages).isNotEmpty();
10601058
Assertions.assertThat(errorCallBackMessages.get(0))
1061-
.isEqualTo(format("com.commercetools.sync.commons.exceptions.InvalidStateDraftException: "
1062-
+ "StateDraft with key: '%s' has invalid state transitions", keyC));
1059+
.isEqualTo(format("StateDraft with key: '%s' has invalid state transitions", keyC));
10631060
Assertions.assertThat(warningCallBackMessages).isEmpty();
10641061
}
10651062

‎src/integration-test/java/com/commercetools/sync/integration/externalsource/types/TypeSyncIT.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,8 @@ void sync_WithoutKey_ShouldExecuteCallbackOnErrorAndIncreaseFailedCounter() {
333333
assertThat(errorMessages)
334334
.hasSize(1)
335335
.hasOnlyOneElementSatisfying(message ->
336-
assertThat(message).isEqualTo("Failed to process type draft without key.")
336+
assertThat(message).isEqualTo("TypeDraft with name: LocalizedString(en -> name_1) doesn't have a key. "
337+
+ "Please make sure all type drafts have keys.")
337338
);
338339

339340
assertThat(exceptions)
@@ -370,7 +371,7 @@ void sync_WithNullDraft_ShouldExecuteCallbackOnErrorAndIncreaseFailedCounter() {
370371
assertThat(errorMessages)
371372
.hasSize(1)
372373
.hasOnlyOneElementSatisfying(message ->
373-
assertThat(message).isEqualTo("Failed to process null type draft.")
374+
assertThat(message).isEqualTo("TypeDraft is null.")
374375
);
375376

376377
assertThat(exceptions)

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

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import static com.commercetools.sync.integration.commons.utils.ITUtils.deleteTypes;
4343
import static com.commercetools.sync.integration.commons.utils.SphereClientUtils.CTP_TARGET_CLIENT;
4444
import static com.commercetools.tests.utils.CompletionStageUtil.executeBlocking;
45-
import static java.lang.String.format;
4645
import static java.util.stream.Collectors.toList;
4746
import static org.assertj.core.api.Assertions.assertThat;
4847
import static org.mockito.ArgumentMatchers.any;
@@ -119,7 +118,8 @@ static void tearDown() {
119118

120119
@Test
121120
void cacheKeysToIds_ShouldCacheCategoryKeysOnlyFirstTime() {
122-
Map<String, String> cache = categoryService.cacheKeysToIds().toCompletableFuture().join();
121+
Map<String, String> cache = categoryService.cacheKeysToIds(Collections.singleton(oldCategoryKey))
122+
.toCompletableFuture().join();
123123
assertThat(cache).hasSize(1);
124124

125125
// Create new category without caching
@@ -132,32 +132,12 @@ void cacheKeysToIds_ShouldCacheCategoryKeysOnlyFirstTime() {
132132

133133
CTP_TARGET_CLIENT.execute(CategoryCreateCommand.of(categoryDraft)).toCompletableFuture().join();
134134

135-
cache = categoryService.cacheKeysToIds().toCompletableFuture().join();
135+
cache = categoryService.cacheKeysToIds(Collections.singleton(oldCategoryKey)).toCompletableFuture().join();
136136
assertThat(cache).hasSize(1);
137137
assertThat(errorCallBackExceptions).isEmpty();
138138
assertThat(errorCallBackMessages).isEmpty();
139139
}
140140

141-
@Test
142-
void cacheKeysToIds_WithTargetCategoriesWithNoKeys_ShouldGiveAWarningAboutKeyNotSetAndNotCacheKey() {
143-
// Create new category without key
144-
final CategoryDraft categoryDraft = CategoryDraftBuilder
145-
.of(LocalizedString.of(Locale.ENGLISH, "classic furniture"),
146-
LocalizedString.of(Locale.ENGLISH, "classic-furniture", Locale.GERMAN, "klassische-moebel"))
147-
.build();
148-
149-
final Category createdCategory = CTP_TARGET_CLIENT.execute(CategoryCreateCommand.of(categoryDraft))
150-
.toCompletableFuture().join();
151-
152-
final Map<String, String> cache = categoryService.cacheKeysToIds().toCompletableFuture().join();
153-
assertThat(cache).hasSize(1);
154-
assertThat(errorCallBackExceptions).isEmpty();
155-
assertThat(errorCallBackMessages).isEmpty();
156-
assertThat(warningCallBackMessages).hasSize(1);
157-
assertThat(warningCallBackMessages.get(0)).isEqualTo(format("Category with id: '%s' has no key set. Keys are"
158-
+ " required for category matching.", createdCategory.getId()));
159-
}
160-
161141
@Test
162142
void fetchMatchingCategoriesByKeys_WithEmptySetOfKeys_ShouldReturnEmptySet() {
163143
final Set<Category> fetchedCategories = categoryService.fetchMatchingCategoriesByKeys(Collections.emptySet())
@@ -515,4 +495,4 @@ void fetchCategory_WithBadGateWayExceptionAlways_ShouldFail() {
515495
.hasFailedWithThrowableThat()
516496
.isExactlyInstanceOf(BadGatewayException.class);
517497
}
518-
}
498+
}

‎src/main/java/com/commercetools/sync/cartdiscounts/CartDiscountSync.java‎

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

3+
import com.commercetools.sync.cartdiscounts.helpers.CartDiscountBatchValidator;
34
import com.commercetools.sync.cartdiscounts.helpers.CartDiscountReferenceResolver;
45
import com.commercetools.sync.cartdiscounts.helpers.CartDiscountSyncStatistics;
56
import com.commercetools.sync.commons.BaseSync;
@@ -31,7 +32,6 @@
3132
import static java.util.function.Function.identity;
3233
import static java.util.stream.Collectors.toMap;
3334
import static java.util.stream.Collectors.toSet;
34-
import static org.apache.commons.lang3.StringUtils.isBlank;
3535

3636
/**
3737
* This class syncs cart discount drafts with the corresponding cart discounts in the CTP project.
@@ -42,14 +42,12 @@ public class CartDiscountSync extends BaseSync<CartDiscountDraft, CartDiscountSy
4242
"Failed to fetch existing cart discounts with keys: '%s'.";
4343
private static final String CTP_CART_DISCOUNT_UPDATE_FAILED =
4444
"Failed to update cart discount with key: '%s'. Reason: %s";
45-
private static final String CART_DISCOUNT_DRAFT_HAS_NO_KEY =
46-
"Failed to process cart discount draft without key.";
47-
private static final String CART_DISCOUNT_DRAFT_IS_NULL =
48-
"Failed to process null cart discount draft.";
4945
private static final String FAILED_TO_PROCESS = "Failed to process the CartDiscountDraft with key:'%s'. Reason: %s";
5046

5147
private final CartDiscountService cartDiscountService;
48+
private final TypeService typeService;
5249
private final CartDiscountReferenceResolver referenceResolver;
50+
private final CartDiscountBatchValidator batchValidator;
5351

5452
/**
5553
* Takes a {@link CartDiscountSyncOptions} to instantiate a new {@link CartDiscountSync} instance
@@ -83,7 +81,9 @@ public CartDiscountSync(@Nonnull final CartDiscountSyncOptions cartDiscountSyncO
8381
@Nonnull final CartDiscountService cartDiscountService) {
8482
super(new CartDiscountSyncStatistics(), cartDiscountSyncOptions);
8583
this.cartDiscountService = cartDiscountService;
86-
this.referenceResolver = new CartDiscountReferenceResolver(cartDiscountSyncOptions, typeService);
84+
this.typeService = typeService;
85+
this.referenceResolver = new CartDiscountReferenceResolver(getSyncOptions(), typeService);
86+
this.batchValidator = new CartDiscountBatchValidator(getSyncOptions(), getStatistics());
8787
}
8888

8989
/**
@@ -105,59 +105,51 @@ protected CompletionStage<CartDiscountSyncStatistics> process(
105105
return syncBatches(batches, completedFuture(statistics));
106106
}
107107

108-
109108
@Override
110109
protected CompletionStage<CartDiscountSyncStatistics> processBatch(@Nonnull final List<CartDiscountDraft> batch) {
111110

112-
final Set<CartDiscountDraft> validCartDiscountDrafts =
113-
batch.stream().filter(this::validateDraft).collect(toSet());
114-
if (validCartDiscountDrafts.isEmpty()) {
111+
final ImmutablePair<Set<CartDiscountDraft>, Set<String>> result =
112+
batchValidator.validateAndCollectReferencedKeys(batch);
113+
114+
final Set<CartDiscountDraft> validDrafts = result.getLeft();
115+
if (validDrafts.isEmpty()) {
115116
statistics.incrementProcessed(batch.size());
116117
return completedFuture(statistics);
117118
}
118119

119-
final Set<String> keys = validCartDiscountDrafts.stream().map(CartDiscountDraft::getKey).collect(toSet());
120+
return typeService
121+
.cacheKeysToIds(result.getRight())
122+
.handle(ImmutablePair::new)
123+
.thenCompose(cachingResponse -> {
124+
final Throwable cachingException = cachingResponse.getValue();
125+
if (cachingException != null) {
126+
handleError("Failed to build a cache of keys to ids.", cachingException,
127+
validDrafts.size());
128+
return CompletableFuture.completedFuture(null);
129+
}
120130

131+
final Set<String> keys = validDrafts.stream().map(CartDiscountDraft::getKey).collect(toSet());
121132

122-
return cartDiscountService
123-
.fetchMatchingCartDiscountsByKeys(keys)
124-
.handle(ImmutablePair::new)
125-
.thenCompose(fetchResponse -> {
126-
final Set<CartDiscount> fetchedCartDiscounts = fetchResponse.getKey();
127-
final Throwable exception = fetchResponse.getValue();
133+
return cartDiscountService
134+
.fetchMatchingCartDiscountsByKeys(keys)
135+
.handle(ImmutablePair::new)
136+
.thenCompose(fetchResponse -> {
137+
final Set<CartDiscount> fetchedCartDiscounts = fetchResponse.getKey();
138+
final Throwable exception = fetchResponse.getValue();
128139

129-
if (exception != null) {
130-
final String errorMessage = format(CTP_CART_DISCOUNT_FETCH_FAILED, keys);
131-
handleError(errorMessage, exception, keys.size());
132-
return CompletableFuture.completedFuture(null);
133-
} else {
134-
return syncBatch(fetchedCartDiscounts, validCartDiscountDrafts);
135-
}
136-
})
137-
.thenApply(ignored -> {
138-
statistics.incrementProcessed(batch.size());
139-
return statistics;
140-
});
141-
}
142-
143-
/**
144-
* Checks if a draft is valid for further processing. If so, then returns {@code true}. Otherwise handles an error
145-
* and returns {@code false}. A valid draft is not {@code null} and its
146-
* key is not empty.
147-
*
148-
* @param cartDiscountDraft nullable draft
149-
* @return boolean that indicate if given {@code draft} is valid for sync
150-
*/
151-
private boolean validateDraft(@Nullable final CartDiscountDraft cartDiscountDraft) {
152-
if (cartDiscountDraft == null) {
153-
handleError(CART_DISCOUNT_DRAFT_IS_NULL, null, 1);
154-
} else if (isBlank(cartDiscountDraft.getKey())) {
155-
handleError(CART_DISCOUNT_DRAFT_HAS_NO_KEY, null, 1);
156-
} else {
157-
return true;
158-
}
159-
160-
return false;
140+
if (exception != null) {
141+
final String errorMessage = format(CTP_CART_DISCOUNT_FETCH_FAILED, keys);
142+
handleError(errorMessage, exception, keys.size());
143+
return CompletableFuture.completedFuture(null);
144+
} else {
145+
return syncBatch(fetchedCartDiscounts, validDrafts);
146+
}
147+
});
148+
})
149+
.thenApply(ignoredResult -> {
150+
statistics.incrementProcessed(batch.size());
151+
return statistics;
152+
});
161153
}
162154

163155
/**

‎src/main/java/com/commercetools/sync/cartdiscounts/helpers/CartDiscountBatchValidator.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package com.commercetools.sync.cartdiscounts.helpers;
2+
3+
import com.commercetools.sync.cartdiscounts.CartDiscountSyncOptions;
4+
import com.commercetools.sync.commons.helpers.BaseBatchValidator;
5+
import io.sphere.sdk.cartdiscounts.CartDiscountDraft;
6+
import io.sphere.sdk.producttypes.ProductTypeDraft;
7+
import org.apache.commons.lang3.tuple.ImmutablePair;
8+
9+
import javax.annotation.Nonnull;
10+
import javax.annotation.Nullable;
11+
import java.util.HashSet;
12+
import java.util.List;
13+
import java.util.Set;
14+
import java.util.stream.Collectors;
15+
16+
import static java.lang.String.format;
17+
import static org.apache.commons.lang3.StringUtils.isBlank;
18+
19+
public class CartDiscountBatchValidator
20+
extends BaseBatchValidator<CartDiscountDraft, CartDiscountSyncOptions, CartDiscountSyncStatistics> {
21+
22+
static final String CART_DISCOUNT_DRAFT_KEY_NOT_SET = "CartDiscountDraft with name: %s doesn't have a key. "
23+
+ "Please make sure all cart discount drafts have keys.";
24+
static final String CART_DISCOUNT_DRAFT_IS_NULL = "CartDiscountDraft is null.";
25+
26+
public CartDiscountBatchValidator(
27+
@Nonnull final CartDiscountSyncOptions syncOptions,
28+
@Nonnull final CartDiscountSyncStatistics syncStatistics) {
29+
30+
super(syncOptions, syncStatistics);
31+
}
32+
33+
/**
34+
* Given the {@link List}&lt;{@link CartDiscountDraft}&gt; of drafts this method attempts to validate
35+
* drafts and collect referenced type keys from the draft
36+
* and return an {@link ImmutablePair}&lt;{@link Set}&lt;{@link CartDiscountDraft}&gt;
37+
* ,{@link Set}&lt;{@link String}&gt;&gt;
38+
* which contains the {@link Set} of valid drafts and referenced type keys.
39+
*
40+
* <p>A valid cart discount draft is one which satisfies the following conditions:
41+
* <ol>
42+
* <li>It is not null</li>
43+
* <li>It has a key which is not blank (null/empty)</li>
44+
* </ol>
45+
*
46+
* @param cartDiscountDrafts the cart discount drafts to validate and collect referenced type keys.
47+
* @return {@link ImmutablePair}&lt;{@link Set}&lt;{@link ProductTypeDraft}&gt;,
48+
* {@link Set}&lt;{@link String}&gt;&gt; which contains the {@link Set} of valid drafts and
49+
* referenced type keys.
50+
*/
51+
@Override
52+
public ImmutablePair<Set<CartDiscountDraft>, Set<String>> validateAndCollectReferencedKeys(
53+
@Nonnull final List<CartDiscountDraft> cartDiscountDrafts) {
54+
final Set<String> typeKeys = new HashSet<>();
55+
final Set<CartDiscountDraft> validDrafts = cartDiscountDrafts
56+
.stream()
57+
.filter(this::isValidCartDiscountDraft)
58+
.peek(cartDiscountDraft ->
59+
collectReferencedKeyFromCustomFieldsDraft(cartDiscountDraft.getCustom(), typeKeys::add))
60+
.collect(Collectors.toSet());
61+
62+
return ImmutablePair.of(validDrafts, typeKeys);
63+
}
64+
65+
private boolean isValidCartDiscountDraft(@Nullable final CartDiscountDraft cartDiscountDraft) {
66+
if (cartDiscountDraft == null) {
67+
handleError(CART_DISCOUNT_DRAFT_IS_NULL);
68+
} else if (isBlank(cartDiscountDraft.getKey())) {
69+
handleError(format(CART_DISCOUNT_DRAFT_KEY_NOT_SET, cartDiscountDraft.getName()));
70+
} else {
71+
return true;
72+
}
73+
74+
return false;
75+
}
76+
}

0 commit comments

Comments
 (0)