Skip to content

Commit 5f741e7

Browse files
ahmetozahmetoz
authored
Handles edge cases for the update actions: setStores and setCustomerNumber. (#601)
* Update setstores action to cover remove and add store together. * adds a warning callback for updating the customer number, which is not possible when it's set. * make public, buildRemoveStoreUpdateActions and buildAddStoreUpdateActions.
1 parent e4ec471 commit 5f741e7

9 files changed

Lines changed: 196 additions & 43 deletions

File tree

‎src/integration-test/java/com/commercetools/sync/integration/externalsource/customers/CustomerSyncIT.java‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import io.sphere.sdk.customers.commands.updateactions.SetLocale;
2828
import io.sphere.sdk.customers.commands.updateactions.SetMiddleName;
2929
import io.sphere.sdk.customers.commands.updateactions.SetSalutation;
30+
import io.sphere.sdk.customers.commands.updateactions.SetStores;
3031
import io.sphere.sdk.customers.commands.updateactions.SetTitle;
3132
import io.sphere.sdk.customers.commands.updateactions.SetVatId;
3233
import io.sphere.sdk.models.Address;
@@ -47,6 +48,7 @@
4748
import java.util.Objects;
4849

4950
import static com.commercetools.sync.commons.asserts.statistics.AssertionsForStatistics.assertThat;
51+
import static com.commercetools.sync.customers.utils.CustomerUpdateActionUtils.CUSTOMER_NUMBER_EXISTS_WARNING;
5052
import static com.commercetools.sync.integration.commons.utils.CustomerGroupITUtils.createCustomerGroup;
5153
import static com.commercetools.sync.integration.commons.utils.CustomerITUtils.createSampleCustomerJohnDoe;
5254
import static com.commercetools.sync.integration.commons.utils.CustomerITUtils.deleteCustomerSyncTestData;
@@ -55,6 +57,7 @@
5557
import static com.commercetools.sync.integration.commons.utils.ITUtils.createCustomFieldsJsonMap;
5658
import static com.commercetools.sync.integration.commons.utils.SphereClientUtils.CTP_TARGET_CLIENT;
5759
import static com.commercetools.sync.integration.commons.utils.StoreITUtils.createStore;
60+
import static java.lang.String.format;
5861
import static java.util.Arrays.asList;
5962
import static java.util.Collections.singletonList;
6063
import static java.util.stream.Collectors.toMap;
@@ -65,6 +68,7 @@ class CustomerSyncIT {
6568
private Customer customerJohnDoe;
6669

6770
private List<String> errorMessages;
71+
private List<String> warningMessages;
6872
private List<Throwable> exceptions;
6973
private List<UpdateAction<Customer>> updateActionList;
7074
private CustomerSync customerSync;
@@ -82,6 +86,7 @@ void setup() {
8286
private void setUpCustomerSync() {
8387
errorMessages = new ArrayList<>();
8488
exceptions = new ArrayList<>();
89+
warningMessages = new ArrayList<>();
8590
updateActionList = new ArrayList<>();
8691

8792
CustomerSyncOptions customerSyncOptions = CustomerSyncOptionsBuilder
@@ -90,6 +95,8 @@ private void setUpCustomerSync() {
9095
errorMessages.add(exception.getMessage());
9196
exceptions.add(exception);
9297
})
98+
.warningCallback((exception, oldResource, newResource)
99+
-> warningMessages.add(exception.getMessage()))
93100
.beforeUpdateCallback((updateActions, customerDraft, customer) -> {
94101
updateActionList.addAll(Objects.requireNonNull(updateActions));
95102
return updateActions;
@@ -112,9 +119,10 @@ void sync_WithSameCustomer_ShouldNotUpdateCustomer() {
112119
.toCompletableFuture()
113120
.join();
114121

115-
assertThat(customerSyncStatistics).hasValues(1, 0, 0, 0);
116122
assertThat(errorMessages).isEmpty();
123+
assertThat(warningMessages).isEmpty();
117124
assertThat(exceptions).isEmpty();
125+
assertThat(customerSyncStatistics).hasValues(1, 0, 0, 0);
118126
assertThat(customerSyncStatistics
119127
.getReportMessage())
120128
.isEqualTo("Summary: 1 customers were processed in total (0 created, 0 updated and 0 failed to sync).");
@@ -141,29 +149,48 @@ void sync_WithNewCustomer_ShouldCreateCustomer() {
141149
.toCompletableFuture()
142150
.join();
143151

144-
assertThat(customerSyncStatistics).hasValues(1, 1, 0, 0);
145152
assertThat(errorMessages).isEmpty();
153+
assertThat(warningMessages).isEmpty();
146154
assertThat(exceptions).isEmpty();
155+
assertThat(updateActionList).isEmpty();
156+
157+
assertThat(customerSyncStatistics).hasValues(1, 1, 0, 0);
147158
assertThat(customerSyncStatistics
148159
.getReportMessage())
149160
.isEqualTo("Summary: 1 customers were processed in total (1 created, 0 updated and 0 failed to sync).");
150161
}
151162

152163
@Test
153164
void sync_WithUpdatedCustomer_ShouldUpdateCustomer() {
165+
final Store storeCologne = createStore(CTP_TARGET_CLIENT, "store-cologne");
154166
final CustomerDraft updatedCustomerDraft =
155167
CustomerDraftBuilder.of(customerDraftJohnDoe)
156-
.email("JOhn@example.com")
168+
.customerNumber("gold-new") // from gold-1, but can not be changed.
169+
.email("john-new@example.com") //from john@example.com
170+
.stores(asList( // store-cologne is added, store-munich is removed
171+
ResourceIdentifier.ofKey(storeCologne.getKey()),
172+
ResourceIdentifier.ofKey("store-hamburg"),
173+
ResourceIdentifier.ofKey("store-berlin")))
157174
.build();
158175

159176
final CustomerSyncStatistics customerSyncStatistics = customerSync
160177
.sync(singletonList(updatedCustomerDraft))
161178
.toCompletableFuture()
162179
.join();
163180

164-
assertThat(customerSyncStatistics).hasValues(1, 0, 1, 0);
165181
assertThat(errorMessages).isEmpty();
182+
assertThat(warningMessages)
183+
.containsExactly(format(CUSTOMER_NUMBER_EXISTS_WARNING, updatedCustomerDraft.getKey(), "gold-1"));
166184
assertThat(exceptions).isEmpty();
185+
assertThat(updateActionList).containsExactly(
186+
ChangeEmail.of("john-new@example.com"),
187+
SetStores.of(asList(
188+
ResourceIdentifier.ofKey("store-cologne"),
189+
ResourceIdentifier.ofKey("store-hamburg"),
190+
ResourceIdentifier.ofKey("store-berlin")))
191+
);
192+
193+
assertThat(customerSyncStatistics).hasValues(1, 0, 1, 0);
167194
assertThat(customerSyncStatistics
168195
.getReportMessage())
169196
.isEqualTo("Summary: 1 customers were processed in total (0 created, 1 updated and 0 failed to sync).");
@@ -214,13 +241,9 @@ void sync_WithUpdatedAllFieldsOfCustomer_ShouldUpdateCustomerWithAllExpectedActi
214241
.join();
215242

216243
assertThat(errorMessages).isEmpty();
244+
assertThat(warningMessages).isEmpty();
217245
assertThat(exceptions).isEmpty();
218246

219-
assertThat(customerSyncStatistics).hasValues(1, 0, 1, 0);
220-
assertThat(customerSyncStatistics
221-
.getReportMessage())
222-
.isEqualTo("Summary: 1 customers were processed in total (0 created, 1 updated and 0 failed to sync).");
223-
224247
final Map<String, String> addressKeyToIdMap =
225248
customerJohnDoe.getAddresses().stream().collect(toMap(Address::getKey, Address::getId));
226249

@@ -246,5 +269,10 @@ void sync_WithUpdatedAllFieldsOfCustomer_ShouldUpdateCustomerWithAllExpectedActi
246269
SetCustomField.ofJson(BOOLEAN_CUSTOM_FIELD_NAME, JsonNodeFactory.instance.booleanNode(false)),
247270
AddStore.of(ResourceIdentifier.ofKey(storeCologne.getKey()))
248271
);
272+
273+
assertThat(customerSyncStatistics).hasValues(1, 0, 1, 0);
274+
assertThat(customerSyncStatistics
275+
.getReportMessage())
276+
.isEqualTo("Summary: 1 customers were processed in total (0 created, 1 updated and 0 failed to sync).");
249277
}
250278
}

‎src/main/java/com/commercetools/sync/customers/commands/updateactions/AddBillingAddressIdWithKey.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import javax.annotation.Nonnull;
77

88
// TODO (JVM-SDK), see: SUPPORT-10260, Address selection by key is not supported yet.
9+
// https://github.com/commercetools/commercetools-jvm-sdk/issues/2071
910
public final class AddBillingAddressIdWithKey extends UpdateActionImpl<Customer> {
1011
private final String addressKey;
1112

‎src/main/java/com/commercetools/sync/customers/commands/updateactions/AddShippingAddressIdWithKey.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import javax.annotation.Nonnull;
77

88
// TODO (JVM-SDK), see: SUPPORT-10260, Address selection by key is not supported yet.
9+
// https://github.com/commercetools/commercetools-jvm-sdk/issues/2071
910
public final class AddShippingAddressIdWithKey extends UpdateActionImpl<Customer> {
1011
private final String addressKey;
1112

‎src/main/java/com/commercetools/sync/customers/commands/updateactions/SetDefaultBillingAddressWithKey.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import javax.annotation.Nullable;
77

88
// TODO (JVM-SDK), see: SUPPORT-10260, Address selection by key is not supported yet.
9+
// https://github.com/commercetools/commercetools-jvm-sdk/issues/2071
910
public final class SetDefaultBillingAddressWithKey extends UpdateActionImpl<Customer> {
1011
private final String addressKey;
1112

‎src/main/java/com/commercetools/sync/customers/commands/updateactions/SetDefaultShippingAddressWithKey.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import javax.annotation.Nullable;
77

88
// TODO (JVM-SDK), see: SUPPORT-10260, Address selection by key is not supported yet.
9+
// https://github.com/commercetools/commercetools-jvm-sdk/issues/2071
910
public final class SetDefaultShippingAddressWithKey extends UpdateActionImpl<Customer> {
1011
private final String addressKey;
1112

‎src/main/java/com/commercetools/sync/customers/utils/CustomerSyncUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public static List<UpdateAction<Customer>> buildActions(
5858
buildSetTitleUpdateAction(oldCustomer, newCustomer),
5959
buildSetSalutationUpdateAction(oldCustomer, newCustomer),
6060
buildSetCustomerGroupUpdateAction(oldCustomer, newCustomer),
61-
buildSetCustomerNumberUpdateAction(oldCustomer, newCustomer),
61+
buildSetCustomerNumberUpdateAction(oldCustomer, newCustomer, syncOptions),
6262
buildSetExternalIdUpdateAction(oldCustomer, newCustomer),
6363
buildSetCompanyNameUpdateAction(oldCustomer, newCustomer),
6464
buildSetDateOfBirthUpdateAction(oldCustomer, newCustomer),

‎src/main/java/com/commercetools/sync/customers/utils/CustomerUpdateActionUtils.java‎

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.commercetools.sync.customers.utils;
22

3+
import com.commercetools.sync.commons.exceptions.SyncException;
4+
import com.commercetools.sync.customers.CustomerSyncOptions;
35
import com.commercetools.sync.customers.commands.updateactions.AddBillingAddressIdWithKey;
46
import com.commercetools.sync.customers.commands.updateactions.AddShippingAddressIdWithKey;
57
import com.commercetools.sync.customers.commands.updateactions.SetDefaultBillingAddressWithKey;
@@ -52,6 +54,7 @@
5254
import static com.commercetools.sync.commons.utils.CommonTypeUpdateActionUtils.buildUpdateActionForReferences;
5355
import static java.lang.String.format;
5456
import static java.util.Collections.emptyList;
57+
import static java.util.Collections.singletonList;
5558
import static java.util.function.Function.identity;
5659
import static java.util.stream.Collectors.toList;
5760
import static java.util.stream.Collectors.toMap;
@@ -60,6 +63,10 @@
6063

6164
public final class CustomerUpdateActionUtils {
6265

66+
public static final String CUSTOMER_NUMBER_EXISTS_WARNING = "Customer with key: \"%s\" has "
67+
+ "already a customer number: \"%s\", once it's set it cannot be changed. "
68+
+ "Hereby, the update action is not created.";
69+
6370
private CustomerUpdateActionUtils() {
6471
}
6572

@@ -183,17 +190,35 @@ public static Optional<UpdateAction<Customer>> buildSetSalutationUpdateAction(
183190
* {@link UpdateAction}. If both {@link Customer} and {@link CustomerDraft} have the same
184191
* {@code customerNumber} values, then no update action is needed and empty optional will be returned.
185192
*
193+
* <p>Note: Customer number should be unique across a project. Once it's set it cannot be changed. For this case,
194+
* warning callback will be triggered and an empty optional will be returned.
195+
*
186196
* @param oldCustomer the customer that should be updated.
187197
* @param newCustomer the customer draft that contains the new customer number.
198+
* @param syncOptions responsible for supplying the sync options to the sync utility method. It is used for
199+
* triggering the warning callback when trying to change an existing customer number.
188200
* @return optional containing update action or empty optional if customer numbers are identical.
189201
*/
190202
@Nonnull
191203
public static Optional<UpdateAction<Customer>> buildSetCustomerNumberUpdateAction(
192204
@Nonnull final Customer oldCustomer,
193-
@Nonnull final CustomerDraft newCustomer) {
205+
@Nonnull final CustomerDraft newCustomer,
206+
@Nonnull final CustomerSyncOptions syncOptions) {
207+
208+
final Optional<UpdateAction<Customer>> setCustomerNumberAction =
209+
buildUpdateAction(oldCustomer.getCustomerNumber(), newCustomer.getCustomerNumber(),
210+
() -> SetCustomerNumber.of(newCustomer.getCustomerNumber()));
211+
212+
if (setCustomerNumberAction.isPresent() && !isBlank(oldCustomer.getCustomerNumber())) {
213+
214+
syncOptions.applyWarningCallback(
215+
new SyncException(format(CUSTOMER_NUMBER_EXISTS_WARNING, oldCustomer.getKey(),
216+
oldCustomer.getCustomerNumber())), oldCustomer, newCustomer);
217+
218+
return Optional.empty();
219+
}
194220

195-
return buildUpdateAction(oldCustomer.getCustomerNumber(), newCustomer.getCustomerNumber(),
196-
() -> SetCustomerNumber.of(newCustomer.getCustomerNumber()));
221+
return setCustomerNumberAction;
197222
}
198223

199224
/**
@@ -319,6 +344,7 @@ private static Referenceable<CustomerGroup> mapResourceIdentifierToReferenceable
319344
}
320345

321346
// TODO (JVM-SDK), see: SUPPORT-10261 SetCustomerGroup could be created with a ResourceIdentifier
347+
// https://github.com/commercetools/commercetools-jvm-sdk/issues/2072
322348
return new ResourceImpl<CustomerGroup>(null, null, null, null) {
323349
@Override
324350
public Reference<CustomerGroup> toReference() {
@@ -349,17 +375,30 @@ public static List<UpdateAction<Customer>> buildStoreUpdateActions(
349375

350376
return buildSetStoreUpdateAction(oldStores, newStores)
351377
.map(Collections::singletonList)
352-
.orElseGet(() -> {
353-
if (oldStores != null && newStores != null) {
354-
final List<UpdateAction<Customer>> updateActions =
355-
buildRemoveStoreUpdateActions(oldStores, newStores);
378+
.orElseGet(() -> prepareStoreActions(oldStores, newStores));
379+
}
380+
381+
private static List<UpdateAction<Customer>> prepareStoreActions(
382+
@Nullable final List<KeyReference<Store>> oldStores,
383+
@Nullable final List<ResourceIdentifier<Store>> newStores) {
356384

357-
updateActions.addAll(buildAddStoreUpdateActions(oldStores, newStores));
358-
return updateActions;
359-
}
385+
if (oldStores != null && newStores != null) {
386+
final List<UpdateAction<Customer>> removeStoreUpdateActions =
387+
buildRemoveStoreUpdateActions(oldStores, newStores);
388+
389+
final List<UpdateAction<Customer>> addStoreUpdateActions =
390+
buildAddStoreUpdateActions(oldStores, newStores);
391+
392+
if (!removeStoreUpdateActions.isEmpty() && !addStoreUpdateActions.isEmpty()) {
393+
return buildSetStoreUpdateAction(newStores)
394+
.map(Collections::singletonList)
395+
.orElseGet(Collections::emptyList);
396+
}
360397

361-
return emptyList();
362-
});
398+
return removeStoreUpdateActions.isEmpty() ? addStoreUpdateActions : removeStoreUpdateActions;
399+
}
400+
401+
return emptyList();
363402
}
364403

365404
/**
@@ -384,13 +423,22 @@ private static Optional<UpdateAction<Customer>> buildSetStoreUpdateAction(
384423
return Optional.of(SetStores.of(emptyList()));
385424
}
386425
} else if (newStores != null && !newStores.isEmpty()) {
387-
final List<ResourceIdentifier<Store>> stores =
388-
newStores.stream()
389-
.filter(Objects::nonNull)
390-
.collect(toList());
391-
if (!stores.isEmpty()) {
392-
return Optional.of(SetStores.of(stores));
393-
}
426+
return buildSetStoreUpdateAction(newStores);
427+
}
428+
429+
return Optional.empty();
430+
}
431+
432+
private static Optional<UpdateAction<Customer>> buildSetStoreUpdateAction(
433+
@Nonnull final List<ResourceIdentifier<Store>> newStores) {
434+
435+
final List<ResourceIdentifier<Store>> stores =
436+
newStores.stream()
437+
.filter(Objects::nonNull)
438+
.collect(toList());
439+
440+
if (!stores.isEmpty()) {
441+
return Optional.of(SetStores.of(stores));
394442
}
395443

396444
return Optional.empty();
@@ -410,7 +458,7 @@ private static Optional<UpdateAction<Customer>> buildSetStoreUpdateAction(
410458
* @return A list containing the update actions or an empty list if the store references are identical.
411459
*/
412460
@Nonnull
413-
private static List<UpdateAction<Customer>> buildRemoveStoreUpdateActions(
461+
public static List<UpdateAction<Customer>> buildRemoveStoreUpdateActions(
414462
@Nonnull final List<KeyReference<Store>> oldStores,
415463
@Nonnull final List<ResourceIdentifier<Store>> newStores) {
416464

@@ -442,7 +490,7 @@ private static List<UpdateAction<Customer>> buildRemoveStoreUpdateActions(
442490
* @return A list containing the update actions or an empty list if the store references are identical.
443491
*/
444492
@Nonnull
445-
private static List<UpdateAction<Customer>> buildAddStoreUpdateActions(
493+
public static List<UpdateAction<Customer>> buildAddStoreUpdateActions(
446494
@Nonnull final List<KeyReference<Store>> oldStores,
447495
@Nonnull final List<ResourceIdentifier<Store>> newStores) {
448496

0 commit comments

Comments
 (0)