Skip to content

Commit 71bf9a6

Browse files
salander85salander85
authored
Merge pull request #1118 from commercetools/DEVX-277_Fix-npe-in-state-reference-resolution-utils
Fix npe in state reference resolution utils
2 parents 7e5f98c + 68a4953 commit 71bf9a6

File tree

10 files changed

+116
-83
lines changed

10 files changed

+116
-83
lines changed

‎src/main/java/com/commercetools/sync/categories/utils/CategoryReferenceResolutionUtils.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,15 +83,7 @@ private static CategoryDraft mapToCategoryDraft(
8383
getResourceIdentifierWithKey(
8484
category.getParent(),
8585
referenceIdToKeyCache,
86-
(id, key) -> {
87-
final CategoryResourceIdentifierBuilder builder =
88-
CategoryResourceIdentifierBuilder.of();
89-
if (id == null) {
90-
return builder.key(key).build();
91-
} else {
92-
return builder.id(id).build();
93-
}
94-
});
86+
(id, key) -> CategoryResourceIdentifierBuilder.of().key(key).id(id).build());
9587
return CategoryDraftBuilder.of()
9688
.key(category.getKey())
9789
.slug(category.getSlug())

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,11 @@ public static CustomFieldsDraft mapToCustomFieldsDraft(
7070
SyncUtils.getResourceIdentifierWithKey(
7171
typeReference,
7272
referenceIdToKeyCache,
73-
(id, key) -> {
74-
final TypeResourceIdentifierBuilder builder =
75-
TypeResourceIdentifierBuilder.of();
76-
if (id == null) {
77-
78-
return builder.key(key).build();
79-
} else {
80-
return builder.id(id).build();
81-
}
82-
}))
73+
(id, key) ->
74+
TypeResourceIdentifierBuilder.of()
75+
.key(key)
76+
.id(id)
77+
.build()))
8378
.orElse(null))
8479
.build())
8580
.orElse(null);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ ResourceIdentifierT getResourceIdentifierWithKey(
7070
@Nullable final String key,
7171
final BiFunction<String, String, ResourceIdentifierT> toResourceIdentifier) {
7272

73-
if (!StringUtils.isEmpty(key)) {
73+
if (!StringUtils.isBlank(key)) {
7474
return toResourceIdentifier.apply(null, key);
7575
}
7676
return toResourceIdentifier.apply(id, null);

‎src/main/java/com/commercetools/sync/inventories/utils/InventoryReferenceResolutionUtils.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,7 @@ private static InventoryEntryDraft mapToInventoryEntryDraft(
8181
getResourceIdentifierWithKey(
8282
inventoryEntry.getSupplyChannel(),
8383
referenceIdToKeyCache,
84-
(id, key) -> {
85-
final ChannelResourceIdentifierBuilder builder =
86-
ChannelResourceIdentifierBuilder.of();
87-
if (id == null) {
88-
return builder.key(key).build();
89-
} else {
90-
return builder.id(id).build();
91-
}
92-
});
84+
(id, key) -> ChannelResourceIdentifierBuilder.of().key(key).id(id).build());
9385
return InventoryEntryDraftBuilder.of()
9486
.sku(inventoryEntry.getSku())
9587
.quantityOnStock(inventoryEntry.getQuantityOnStock())

‎src/main/java/com/commercetools/sync/products/utils/PriceUtils.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,7 @@ private static ChannelResourceIdentifier createChannelResourceIdentifier(
6767
getResourceIdentifierWithKey(
6868
channelReference,
6969
referenceIdToKeyCache,
70-
(id, key) -> {
71-
final ChannelResourceIdentifierBuilder builder =
72-
ChannelResourceIdentifierBuilder.of();
73-
if (id == null) {
74-
return builder.key(key).build();
75-
} else {
76-
return builder.id(id).build();
77-
}
78-
});
70+
(id, key) -> ChannelResourceIdentifierBuilder.of().key(key).id(id).build());
7971
} else if (channelReference != null) {
8072
channelResourceIdentifier =
8173
ChannelResourceIdentifierBuilder.of().id(channelReference.getId()).build();

‎src/main/java/com/commercetools/sync/products/utils/ProductReferenceResolutionUtils.java‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -149,41 +149,19 @@ public static List<ProductDraft> mapToProductDrafts(
149149
getResourceIdentifierWithKey(
150150
product.getProductType(),
151151
referenceIdToKeyCache,
152-
(id, key) -> {
153-
final ProductTypeResourceIdentifierBuilder builder =
154-
ProductTypeResourceIdentifierBuilder.of();
155-
if (id == null) {
156-
return builder.key(key).build();
157-
} else {
158-
return builder.id(id).build();
159-
}
160-
});
152+
(id, key) ->
153+
ProductTypeResourceIdentifierBuilder.of().id(id).key(key).build());
161154
final TaxCategoryResourceIdentifier taxCategoryResourceIdentifier =
162155
getResourceIdentifierWithKey(
163156
product.getTaxCategory(),
164157
referenceIdToKeyCache,
165-
(id, key) -> {
166-
final TaxCategoryResourceIdentifierBuilder builder =
167-
TaxCategoryResourceIdentifierBuilder.of();
168-
if (id == null) {
169-
return builder.key(key).build();
170-
} else {
171-
return builder.id(id).build();
172-
}
173-
});
158+
(id, key) ->
159+
TaxCategoryResourceIdentifierBuilder.of().key(key).id(id).build());
174160
final StateResourceIdentifier stateResourceIdentifier =
175161
getResourceIdentifierWithKey(
176162
product.getState(),
177163
referenceIdToKeyCache,
178-
(id, key) -> {
179-
final StateResourceIdentifierBuilder builder =
180-
StateResourceIdentifierBuilder.of();
181-
if (id == null) {
182-
return builder.key(key).build();
183-
} else {
184-
return builder.id(id).build();
185-
}
186-
});
164+
(id, key) -> StateResourceIdentifierBuilder.of().key(key).id(id).build());
187165
return productDraftBuilder
188166
.masterVariant(masterVariantDraftWithKeys)
189167
.variants(variantDraftsWithKeys)

‎src/main/java/com/commercetools/sync/states/utils/StateReferenceResolutionUtils.java‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,37 @@ public static List<StateDraft> mapToStateDrafts(
6565
state -> {
6666
final List<StateResourceIdentifier> newTransitions =
6767
replaceTransitionIdsWithKeys(state, referenceIdToKeyCache);
68-
return StateDraftBuilder.of()
69-
.key(state.getKey())
70-
.type(state.getType())
71-
.name(state.getName())
72-
.description(state.getDescription())
73-
.initial(state.getInitial())
74-
.roles(state.getRoles())
75-
.transitions(newTransitions)
76-
.build();
68+
return getStateDraft(state, newTransitions);
7769
})
7870
.collect(Collectors.toList());
7971
}
8072

73+
/**
74+
* Creates a new {@link StateDraft} from given {@link State} and transitions as {@link
75+
* java.util.List}&lt; {@link StateResourceIdentifier}&gt;
76+
*
77+
* @param state - template state to build the draft from
78+
* @param newTransitions - transformed list of state resource identifiers
79+
* @return a new {@link StateDraft} with all fields copied from the {@param state} and transitions
80+
* set {@param newTransitions} - it will return empty StateDraft if key or type are missing.
81+
*/
82+
private static StateDraft getStateDraft(
83+
State state, List<StateResourceIdentifier> newTransitions) {
84+
if (state.getKey() != null && state.getType() != null) {
85+
return StateDraftBuilder.of()
86+
.key(state.getKey())
87+
.type(state.getType())
88+
.name(state.getName())
89+
.description(state.getDescription())
90+
.initial(state.getInitial())
91+
.roles(state.getRoles())
92+
.transitions(newTransitions)
93+
.build();
94+
} else {
95+
return StateDraft.of();
96+
}
97+
}
98+
8199
@SuppressWarnings("PMD.ReturnEmptyCollectionRatherThanNull")
82100
private static List<StateResourceIdentifier> replaceTransitionIdsWithKeys(
83101
@Nonnull final State state, @Nonnull final ReferenceIdToKeyCache referenceIdToKeyCache) {

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.commercetools.sync.commons.utils;
22

3+
import static com.commercetools.sync.services.impl.BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER;
34
import static java.lang.String.format;
45
import static org.assertj.core.api.Assertions.assertThat;
56

@@ -161,4 +162,40 @@ void getResourceIdentifierWithKey_WithNullReference_ShouldReturnNull() {
161162
(id, key) -> CategoryResourceIdentifierBuilder.of().id(id).key(key).build());
162163
assertThat(resourceIdentifier).isNull();
163164
}
165+
166+
@Test
167+
void getResourceIdentifierWithKey_WithNotCachedReference_ShouldReturnResourceIdentifierWithId() {
168+
final String categoryId = UUID.randomUUID().toString();
169+
170+
final CategoryReference categoryReference =
171+
CategoryReferenceBuilder.of().id(categoryId).build();
172+
173+
final CategoryResourceIdentifier resourceIdentifier =
174+
SyncUtils.getResourceIdentifierWithKey(
175+
categoryReference,
176+
referenceIdToKeyCache,
177+
(id, key) -> CategoryResourceIdentifierBuilder.of().id(id).key(key).build());
178+
179+
assertThat(resourceIdentifier).isNotNull();
180+
assertThat(resourceIdentifier.getId()).isEqualTo(categoryId);
181+
}
182+
183+
@Test
184+
void
185+
getResourceIdentifierWithKey_WithCachedReferenceIsEmptyPlaceholder_ShouldReturnResourceIdentifierWithKeyPlaceholder() {
186+
final String categoryId = UUID.randomUUID().toString();
187+
referenceIdToKeyCache.add(categoryId, KEY_IS_NOT_SET_PLACE_HOLDER);
188+
189+
final CategoryReference categoryReference =
190+
CategoryReferenceBuilder.of().id(categoryId).build();
191+
192+
final CategoryResourceIdentifier resourceIdentifier =
193+
SyncUtils.getResourceIdentifierWithKey(
194+
categoryReference,
195+
referenceIdToKeyCache,
196+
(id, key) -> CategoryResourceIdentifierBuilder.of().id(id).key(key).build());
197+
198+
assertThat(resourceIdentifier).isNotNull();
199+
assertThat(resourceIdentifier.getKey()).isEqualTo(KEY_IS_NOT_SET_PLACE_HOLDER);
200+
}
164201
}

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

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

3+
import static com.commercetools.sync.services.impl.BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER;
34
import static java.util.Arrays.asList;
45
import static java.util.Collections.singletonList;
56
import static org.assertj.core.api.Assertions.assertThat;
@@ -25,7 +26,6 @@
2526
import com.commercetools.sync.commons.utils.ReferenceIdToKeyCache;
2627
import com.commercetools.sync.commons.utils.TestUtils;
2728
import com.commercetools.sync.products.ProductSyncMockUtils;
28-
import com.commercetools.sync.services.impl.BaseTransformServiceImpl;
2929
import com.fasterxml.jackson.core.JsonProcessingException;
3030
import com.fasterxml.jackson.databind.JsonNode;
3131
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -439,9 +439,9 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
439439
.hasValueSatisfying(
440440
productDraft -> {
441441
assertThat(productDraft.getProductType().getKey())
442-
.isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
442+
.isEqualTo(KEY_IS_NOT_SET_PLACE_HOLDER);
443443
assertThat(productDraft.getTaxCategory().getKey())
444-
.isEqualTo(BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
444+
.isEqualTo(KEY_IS_NOT_SET_PLACE_HOLDER);
445445
});
446446
}
447447

@@ -451,9 +451,8 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
451451
throws Exception {
452452
// preparation
453453
final ProjectApiRoot sourceClient = mock(ProjectApiRoot.class);
454-
referenceIdToKeyCache.add(
455-
"cda0dbf7-b42e-40bf-8453-241d5b587f93",
456-
BaseTransformServiceImpl.KEY_IS_NOT_SET_PLACE_HOLDER);
454+
referenceIdToKeyCache.add("cda0dbf7-b42e-40bf-8453-241d5b587f93", KEY_IS_NOT_SET_PLACE_HOLDER);
455+
referenceIdToKeyCache.add("ste95fb-2282-4f9a-8747-fbe440e02dcs0", KEY_IS_NOT_SET_PLACE_HOLDER);
457456
final List<ProductProjection> productPage =
458457
Arrays.asList(
459458
ProductSyncMockUtils.createProductFromJson("product-with-unresolved-references.json"));
@@ -482,16 +481,17 @@ void transform_ProductReferences_ShouldReplaceReferencesIdsWithKeysAndMapToProdu
482481
.join();
483482

484483
// assertions
485-
486484
final Optional<ProductDraft> productKey1 =
487485
productsResolved.stream()
488486
.filter(productDraft -> "productKeyResolved".equals(productDraft.getKey()))
489487
.findFirst();
490488

491489
assertThat(productKey1)
492490
.hasValueSatisfying(
493-
productDraft ->
494-
assertThat(productDraft.getProductType().getKey()).isEqualTo("productTypeKey"));
491+
productDraft -> {
492+
assertThat(productDraft.getProductType().getKey()).isEqualTo("productTypeKey");
493+
assertThat(productDraft.getState().getKey()).isEqualTo(KEY_IS_NOT_SET_PLACE_HOLDER);
494+
});
495495
}
496496

497497
@Test

‎src/test/java/com/commercetools/sync/states/utils/StateReferenceResolutionUtilsTest.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,35 @@ void mapToStateDrafts_WithNullReferences_ShouldNotFail() {
120120
assertThat(referenceReplacedDrafts.get(0).getTransitions()).isEqualTo(null);
121121
}
122122

123+
@Test
124+
void mapToStateDrafts_WithMissingRequiredFields_ShouldNotFailAndReturnEmptyDraft() {
125+
// preparation
126+
final State mockState = mock(State.class);
127+
when(mockState.getTransitions()).thenReturn(null);
128+
129+
// asserts
130+
assertThat(
131+
StateReferenceResolutionUtils.mapToStateDrafts(
132+
List.of(mockState), referenceIdToKeyCache)
133+
.get(0))
134+
.isEqualTo(StateDraft.of());
135+
136+
when(mockState.getKey()).thenReturn("Any key");
137+
assertThat(
138+
StateReferenceResolutionUtils.mapToStateDrafts(
139+
List.of(mockState), referenceIdToKeyCache)
140+
.get(0))
141+
.isEqualTo(StateDraft.of());
142+
143+
when(mockState.getKey()).thenReturn(null);
144+
when(mockState.getType()).thenReturn(StateTypeEnum.LINE_ITEM_STATE);
145+
assertThat(
146+
StateReferenceResolutionUtils.mapToStateDrafts(
147+
List.of(mockState), referenceIdToKeyCache)
148+
.get(0))
149+
.isEqualTo(StateDraft.of());
150+
}
151+
123152
@Nonnull
124153
private static State getStateMock(@Nonnull final String key) {
125154
final State state = mock(State.class);

0 commit comments

Comments
 (0)