From bd2bf070b5ba49d715e63f2356bd85f8d6874f43 Mon Sep 17 00:00:00 2001 From: salander85 Date: Tue, 31 Oct 2023 12:55:53 +0100 Subject: [PATCH 1/2] Migrate type syncer --- .../project/sync/product/ProductSyncer.java | 9 +- .../project/sync/type/TypeSyncer.java | 20 +- .../project/sync/util/SyncUtils.java | 9 + .../project/sync/type/TypeSyncerTest.java | 211 +++++++++++------- .../project/sync/util/SyncUtilsTest.java | 19 ++ 5 files changed, 172 insertions(+), 96 deletions(-) diff --git a/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java b/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java index eaf031d5..dcf12769 100644 --- a/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java +++ b/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java @@ -1,6 +1,7 @@ package com.commercetools.project.sync.product; import static com.commercetools.project.sync.util.SyncUtils.IDENTIFIER_NOT_PRESENT; +import static com.commercetools.project.sync.util.SyncUtils.getCompletionExceptionCause; import static com.commercetools.project.sync.util.SyncUtils.logErrorCallback; import static com.commercetools.project.sync.util.SyncUtils.logWarningCallback; import static com.commercetools.sync.products.utils.ProductTransformUtils.toProductDrafts; @@ -171,14 +172,6 @@ private ProductVariantDraft createProductVariantDraftWithoutDiscounted( return ProductVariantDraftBuilder.of(productVariantDraft).prices(priceDrafts).build(); } - @Nonnull - private static Throwable getCompletionExceptionCause(@Nonnull final Throwable exception) { - if (exception instanceof CompletionException) { - return getCompletionExceptionCause(exception.getCause()); - } - return exception; - } - @Nonnull @Override protected ByProjectKeyProductProjectionsGet getQuery() { diff --git a/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java b/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java index 84e95315..05f69221 100644 --- a/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java +++ b/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java @@ -1,5 +1,6 @@ package com.commercetools.project.sync.type; +import static com.commercetools.project.sync.util.SyncUtils.getCompletionExceptionCause; import static com.commercetools.project.sync.util.SyncUtils.logErrorCallback; import static com.commercetools.project.sync.util.SyncUtils.logWarningCallback; @@ -27,6 +28,9 @@ import java.util.concurrent.CompletionStage; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import io.vrap.rmf.base.client.utils.CompletableFutureUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -96,11 +100,21 @@ protected Logger getLoggerInstance() { @Nonnull @Override protected CompletionStage> transform(@Nonnull final List page) { - return CompletableFuture.completedFuture( - page.stream().map(TypeSyncer::typeToDraft).collect(Collectors.toList())); + return CompletableFuture.supplyAsync( + () -> page.stream().map(TypeSyncer::typeToDraft).collect(Collectors.toList())) + .handle( + ((typeDrafts, throwable) -> { + if (throwable != null) { + if (LOGGER.isWarnEnabled()) { + LOGGER.warn(throwable.getMessage(), getCompletionExceptionCause(throwable)); + } + return List.of(); + } + return typeDrafts; + })); } - @Nonnull + @Nullable private static TypeDraft typeToDraft(@Nonnull final Type type) { return TypeDraftBuilder.of() .key(type.getKey()) diff --git a/src/main/java/com/commercetools/project/sync/util/SyncUtils.java b/src/main/java/com/commercetools/project/sync/util/SyncUtils.java index 66196a07..20c77811 100644 --- a/src/main/java/com/commercetools/project/sync/util/SyncUtils.java +++ b/src/main/java/com/commercetools/project/sync/util/SyncUtils.java @@ -11,6 +11,7 @@ import com.commercetools.sync.commons.exceptions.SyncException; import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletionException; import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -112,6 +113,14 @@ public static void logWarningCallback( } } + @Nonnull + public static Throwable getCompletionExceptionCause(@Nonnull final Throwable exception) { + if (exception instanceof CompletionException) { + return getCompletionExceptionCause(exception.getCause()); + } + return exception; + } + @Nonnull public static String buildLastSyncTimestampContainerName( @Nonnull final String syncModuleName, @Nullable final String runnerName) { diff --git a/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java b/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java index 218162e4..1e157aff 100644 --- a/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java +++ b/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java @@ -1,11 +1,35 @@ package com.commercetools.project.sync.type; +import static com.commercetools.project.sync.util.TestUtils.getMockedClock; +import static com.commercetools.project.sync.util.TestUtils.readObjectFromResource; +import static java.lang.String.format; +import static java.util.stream.Collectors.toList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.commercetools.api.client.ByProjectKeyTypesGet; +import com.commercetools.api.client.ProjectApiRoot; +import com.commercetools.api.defaultconfig.ApiRootBuilder; +import com.commercetools.api.models.type.Type; +import com.commercetools.api.models.type.TypeDraft; +import com.commercetools.api.models.type.TypeDraftBuilder; +import com.commercetools.api.models.type.TypePagedQueryResponse; +import com.commercetools.api.models.type.TypePagedQueryResponseBuilder; +import com.commercetools.sync.types.TypeSync; +import io.vrap.rmf.base.client.ApiHttpResponse; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import uk.org.lidalia.slf4jtest.LoggingEvent; import uk.org.lidalia.slf4jtest.TestLogger; import uk.org.lidalia.slf4jtest.TestLoggerFactory; -// These tests aren't migrated -// TODO: Migrate tests class TypeSyncerTest { private final TestLogger syncerTestLogger = TestLoggerFactory.getTestLogger(TypeSyncer.class); @@ -14,87 +38,104 @@ void setup() { syncerTestLogger.clearAll(); } - // @Test - // void of_ShouldCreateTypeSyncerInstance() { - // // test - // final TypeSyncer typeSyncer = - // TypeSyncer.of(mock(SphereClient.class), mock(SphereClient.class), getMockedClock()); - // - // // assertions - // assertThat(typeSyncer).isNotNull(); - // assertThat(typeSyncer.getQuery()).isEqualTo(TypeQuery.of()); - // assertThat(typeSyncer.getSync()).isInstanceOf(TypeSync.class); - // } - // - // @Test - // void transform_ShouldConvertResourcesToDrafts() { - // // preparation - // final TypeSyncer typeSyncer = - // TypeSyncer.of(mock(SphereClient.class), mock(SphereClient.class), getMockedClock()); - // final List typePage = - // asList( - // readObjectFromResource("type-key-1.json", Type.class), - // readObjectFromResource("type-key-2.json", Type.class)); - // - // // test - // final CompletionStage> draftsFromPageStage = typeSyncer.transform(typePage); - // - // // assertions - // assertThat(draftsFromPageStage) - // .isCompletedWithValue( - // typePage.stream() - // .map( - // type -> - // TypeDraftBuilder.of( - // type.getKey(), type.getName(), type.getResourceTypeIds()) - // .fieldDefinitions(type.getFieldDefinitions()) - // .description(type.getDescription())) - // .map(TypeDraftBuilder::build) - // .collect(toList())); - // } - // - // @Test - // void getQuery_ShouldBuildTypeQuery() { - // // preparation - // final TypeSyncer typeSyncer = - // TypeSyncer.of(mock(SphereClient.class), mock(SphereClient.class), getMockedClock()); - // - // // test - // final TypeQuery query = typeSyncer.getQuery(); - // - // // assertion - // assertThat(query).isEqualTo(TypeQuery.of()); - // } - // - // @Test - // void syncWithError_WhenNoKeyIsProvided_ShouldCallErrorCallback() { - // // preparation - // final SphereClient sourceClient = mock(SphereClient.class); - // final SphereClient targetClient = mock(SphereClient.class); - // when(sourceClient.getConfig()).thenReturn(SphereApiConfig.of("source-project")); - // when(targetClient.getConfig()).thenReturn(SphereApiConfig.of("target-project")); - // - // final List typePage = - // Collections.singletonList(readObjectFromResource("type-without-key.json", Type.class)); - // - // final PagedQueryResult pagedQueryResult = mock(PagedQueryResult.class); - // when(pagedQueryResult.getResults()).thenReturn(typePage); - // when(sourceClient.execute(any(TypeQuery.class))) - // .thenReturn(CompletableFuture.completedFuture(pagedQueryResult)); - // - // // test - // final TypeSyncer typeSyncer = TypeSyncer.of(sourceClient, targetClient, mock(Clock.class)); - // typeSyncer.sync(null, true).toCompletableFuture().join(); - // - // // assertion - // final LoggingEvent errorLog = syncerTestLogger.getAllLoggingEvents().get(1); - // assertThat(errorLog.getMessage()) - // .isEqualTo( - // "Error when trying to sync type. Existing key: <>. Update actions: - // []"); - // assertThat(errorLog.getThrowable().get().getMessage()) - // .isEqualTo( - // "TypeDraft with name: LocalizedString(en -> typeName) doesn't have a key. Please - // make sure all type drafts have keys."); - // } + @Test + void of_ShouldCreateTypeSyncerInstance() { + // preparation + final ProjectApiRoot sourceClient = mock(ProjectApiRoot.class); + when(sourceClient.types()).thenReturn(mock()); + when(sourceClient.types().get()).thenReturn(mock()); + + // test + final TypeSyncer typeSyncer = + TypeSyncer.of(sourceClient, mock(ProjectApiRoot.class), getMockedClock()); + + // assertions + assertThat(typeSyncer).isNotNull(); + assertThat(typeSyncer.getSync()).isInstanceOf(TypeSync.class); + } + + @Test + void transform_ShouldConvertResourcesToDrafts() { + // preparation + final TypeSyncer typeSyncer = + TypeSyncer.of(mock(ProjectApiRoot.class), mock(ProjectApiRoot.class), getMockedClock()); + final List typePage = + List.of( + readObjectFromResource("type-key-1.json", Type.class), + readObjectFromResource("type-key-2.json", Type.class)); + + // test + final CompletionStage> draftsFromPageStage = typeSyncer.transform(typePage); + + // assertions + assertThat(draftsFromPageStage) + .isCompletedWithValue( + typePage.stream() + .map( + type -> + TypeDraftBuilder.of() + .key(type.getKey()) + .name(type.getName()) + .resourceTypeIds(type.getResourceTypeIds()) + .fieldDefinitions(type.getFieldDefinitions()) + .description(type.getDescription())) + .map(TypeDraftBuilder::build) + .collect(toList())); + } + + @Test + void getQuery_ShouldBuildTypeQuery() { + // preparation + final ProjectApiRoot apiRoot = + ApiRootBuilder.of().withApiBaseUrl("baseURl").build("testProjectKey"); + final TypeSyncer typeSyncer = + TypeSyncer.of(apiRoot, mock(ProjectApiRoot.class), getMockedClock()); + + // test + final ByProjectKeyTypesGet query = typeSyncer.getQuery(); + + // assertion + assertThat(query).isInstanceOf(ByProjectKeyTypesGet.class); + } + + @Test + void syncWithError_WhenNoKeyIsProvided_ShouldCallErrorCallback() { + // preparation + final ProjectApiRoot sourceClient = mock(ProjectApiRoot.class); + final List typePage = + List.of(readObjectFromResource("type-without-key.json", Type.class)); + final TypePagedQueryResponse response = + TypePagedQueryResponseBuilder.of() + .results(typePage) + .limit(20L) + .offset(0L) + .count(1L) + .build(); + + final ApiHttpResponse apiHttpResponse = mock(ApiHttpResponse.class); + when(apiHttpResponse.getBody()).thenReturn(response); + final ByProjectKeyTypesGet byProjectKeyTypesGet = mock(ByProjectKeyTypesGet.class); + when(byProjectKeyTypesGet.execute()) + .thenReturn(CompletableFuture.completedFuture(apiHttpResponse)); + when(byProjectKeyTypesGet.withLimit(anyInt())).thenReturn(byProjectKeyTypesGet); + when(byProjectKeyTypesGet.withWithTotal(anyBoolean())).thenReturn(byProjectKeyTypesGet); + when(byProjectKeyTypesGet.withSort(anyString())).thenReturn(byProjectKeyTypesGet); + when(sourceClient.types()).thenReturn(mock()); + when(sourceClient.types().get()).thenReturn(byProjectKeyTypesGet); + + // test + final TypeSyncer typeSyncer = + TypeSyncer.of(sourceClient, mock(ProjectApiRoot.class), getMockedClock()); + typeSyncer.sync(null, true).toCompletableFuture().join(); + + // assertion + final LoggingEvent errorLog = syncerTestLogger.getAllLoggingEvents().get(1); + assertThat(errorLog.getMessage()) + .isEqualTo( + "Error when trying to sync types. Existing key: <>. Update actions: []"); + assertThat(errorLog.getThrowable().get().getMessage()) + .containsIgnoringCase("TypeDraft: key is missing"); + } + + //TODO: Add test for batchValidator if possible / and caching empty keys? } diff --git a/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java b/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java index 84224666..303c3686 100644 --- a/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java +++ b/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java @@ -1,6 +1,7 @@ package com.commercetools.project.sync.util; import static com.commercetools.project.sync.util.SyncUtils.*; +import static com.commercetools.project.sync.util.TestUtils.createBadGatewayException; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -12,6 +13,9 @@ import com.commercetools.sync.types.TypeSync; import java.util.Arrays; import java.util.Optional; +import java.util.concurrent.CompletionException; + +import io.vrap.rmf.base.client.error.BadGatewayException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import uk.org.lidalia.slf4jtest.LoggingEvent; @@ -175,4 +179,19 @@ void logWarningCallbackWithResource_ShouldLogWarningWithCorrectMessage() { assertThat(loggingEvent.getThrowable().isPresent()).isTrue(); assertThat(loggingEvent.getThrowable().get()).isInstanceOf(SyncException.class); } + + @Test + void getCompletionExceptionCause_WithCompletionException_ShouldReturnDifferentException() { + final BadGatewayException wrappedException = createBadGatewayException(); + final CompletionException completionException = new CompletionException(wrappedException); + + assertThat(getCompletionExceptionCause(completionException)).isEqualTo(wrappedException); + } + + @Test + void getCompletionExceptionCause_WithBadGatewayException_ShouldReturnException() { + final BadGatewayException badGatewayException = createBadGatewayException(); + + assertThat(getCompletionExceptionCause(badGatewayException)).isEqualTo(badGatewayException); + } } From 45ccb69effacd972d3e2f83521927422fe6df5b0 Mon Sep 17 00:00:00 2001 From: salander85 Date: Tue, 31 Oct 2023 13:30:06 +0100 Subject: [PATCH 2/2] Test type-sync fails with error callback --- .../project/sync/product/ProductSyncer.java | 1 - .../project/sync/type/TypeSyncer.java | 2 - .../project/sync/type/TypeSyncerTest.java | 44 +++++++++++++++---- .../project/sync/util/SyncUtilsTest.java | 3 +- 4 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java b/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java index dcf12769..c046ec5e 100644 --- a/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java +++ b/src/main/java/com/commercetools/project/sync/product/ProductSyncer.java @@ -34,7 +34,6 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; import java.util.stream.Collectors; import javax.annotation.Nonnull; diff --git a/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java b/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java index 05f69221..a35d5b5e 100644 --- a/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java +++ b/src/main/java/com/commercetools/project/sync/type/TypeSyncer.java @@ -29,8 +29,6 @@ import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; - -import io.vrap.rmf.base.client.utils.CompletableFutureUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java b/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java index 1e157aff..3f6b5a37 100644 --- a/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java +++ b/src/test/java/com/commercetools/project/sync/type/TypeSyncerTest.java @@ -21,6 +21,7 @@ import com.commercetools.api.models.type.TypePagedQueryResponseBuilder; import com.commercetools.sync.types.TypeSync; import io.vrap.rmf.base.client.ApiHttpResponse; +import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; @@ -99,14 +100,40 @@ void getQuery_ShouldBuildTypeQuery() { } @Test - void syncWithError_WhenNoKeyIsProvided_ShouldCallErrorCallback() { + void transform_WhenNoKeyIsProvided_ShouldLogErrorAndContinue() { // preparation final ProjectApiRoot sourceClient = mock(ProjectApiRoot.class); final List typePage = List.of(readObjectFromResource("type-without-key.json", Type.class)); + + // test + final TypeSyncer typeSyncer = + TypeSyncer.of(sourceClient, mock(ProjectApiRoot.class), getMockedClock()); + final CompletableFuture> typeDrafts = + typeSyncer.transform(typePage).toCompletableFuture(); + + // assertion + assertThat(typeDrafts).isCompletedWithValue(Collections.emptyList()); + assertThat(syncerTestLogger.getAllLoggingEvents()) + .anySatisfy( + loggingEvent -> { + assertThat(loggingEvent.getMessage()).contains("TypeDraft: key is missing"); + assertThat(loggingEvent.getThrowable().isPresent()).isTrue(); + assertThat(loggingEvent.getThrowable().get()) + .isInstanceOf(NullPointerException.class); + }); + } + + @Test + void sync_whenKeyIsBlank_shouldCallErrorCallback() { + // preparation + final ProjectApiRoot sourceClient = mock(ProjectApiRoot.class); + Type type = readObjectFromResource("type-key-1.json", Type.class); + type.setKey(""); + final TypePagedQueryResponse response = TypePagedQueryResponseBuilder.of() - .results(typePage) + .results(List.of(type)) .limit(20L) .offset(0L) .count(1L) @@ -126,16 +153,17 @@ void syncWithError_WhenNoKeyIsProvided_ShouldCallErrorCallback() { // test final TypeSyncer typeSyncer = TypeSyncer.of(sourceClient, mock(ProjectApiRoot.class), getMockedClock()); - typeSyncer.sync(null, true).toCompletableFuture().join(); + typeSyncer.sync(null, true); - // assertion + // assertions final LoggingEvent errorLog = syncerTestLogger.getAllLoggingEvents().get(1); assertThat(errorLog.getMessage()) .isEqualTo( - "Error when trying to sync types. Existing key: <>. Update actions: []"); + "Error when trying to sync type. Existing key: <>. Update actions: []"); assertThat(errorLog.getThrowable().get().getMessage()) - .containsIgnoringCase("TypeDraft: key is missing"); + .isEqualTo( + format( + "TypeDraft with name: %s doesn't have a key. Please make sure all type drafts have keys.", + type.getName())); } - - //TODO: Add test for batchValidator if possible / and caching empty keys? } diff --git a/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java b/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java index 303c3686..3c320ebe 100644 --- a/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java +++ b/src/test/java/com/commercetools/project/sync/util/SyncUtilsTest.java @@ -11,11 +11,10 @@ import com.commercetools.sync.commons.exceptions.SyncException; import com.commercetools.sync.products.ProductSync; import com.commercetools.sync.types.TypeSync; +import io.vrap.rmf.base.client.error.BadGatewayException; import java.util.Arrays; import java.util.Optional; import java.util.concurrent.CompletionException; - -import io.vrap.rmf.base.client.error.BadGatewayException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import uk.org.lidalia.slf4jtest.LoggingEvent;