#300: PR review changes, mostly documentation and indentions.

docs/usage/TYPE_SYNC.md

@@ -1,6 +1,6 @@
 # commercetools type sync
 
-Utility which provides API for building CTP type update actions and type synchronisation.
+Utility which provides an API for building CTP type update actions and type synchronisation.
 
 <!-- START doctoc generated TOC please keep comment here to allow auto update -->
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
@@ -30,7 +30,7 @@ matched.
     - Retries on 5xx errors with a retry strategy. This can be achieved by decorating the `sphereClient` with the
    [RetrySphereClientDecorator](http://commercetools.github.io/commercetools-jvm-sdk/apidocs/io/sphere/sdk/client/RetrySphereClientDecorator.html)
 
-   You can use the same client instantiating used in the integration tests for this library found
+   You can use the same client instance in the integration tests for this library found
    [here](/src/main/java/com/commercetools/sync/commons/utils/ClientConfigurationUtils.java#L45).
 
 4. After the `sphereClient` is setup, a `TypeSyncOptions` should be be built as follows:
@@ -109,12 +109,13 @@ Utility methods provided by the library to compare the specific fields of a `Typ
 ````java
 Optional<UpdateAction<Type>> updateAction = TypeUpdateActionUtils.buildChangeNameAction(oldType, typeDraft);
 ````
-More examples of those utils for different fields can be found [here](/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java).
-
+More examples of those utils for different types can be found [here](/src/test/java/com/commercetools/sync/types/utils/TypeUpdateActionUtilsTest.java).
+and field definitions can be found [here](/src/test/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtilsTest.java).
 
 ## Caveats
 
 1. Types are either created or updated. Currently the tool does not support type deletion.
 2. Updating the label of enum values and localized enum values of field definition is not supported yet.
 3. Removing the enum values from the field definition is not supported yet.
-4. Updating the input hint of field definition is not supported yet.
+4. Updating the input hint of field definition is not supported yet.
+5. Updating the reference and field definition type is not supported yet.

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

@@ -465,7 +465,8 @@ public void sync_WithoutKey_ShouldExecuteCallbackOnErrorAndIncreaseFailedCounter
                 .sync(singletonList(newTypeDraft))
                 .toCompletableFuture().join();
 
-        verify(spyTypeSyncOptions).applyErrorCallback("Failed to process type draft without key.", null);
+        verify(spyTypeSyncOptions)
+                .applyErrorCallback("Failed to process type draft without key.", null);
 
         AssertionsForStatistics.assertThat(typeSyncStatistics).hasValues(1, 0, 0, 1);
     }
@@ -684,7 +685,8 @@ public void sync_WithSeveralBatches_ShouldReturnProperStatistics() {
                 .sync(typeDrafts)
                 .toCompletableFuture().join();
 
-        AssertionsForStatistics.assertThat(typeSyncStatistics).hasValues(100, 100, 0, 0);
+        AssertionsForStatistics.assertThat(typeSyncStatistics)
+                .hasValues(100, 100, 0, 0);
     }
 
     @Test
@@ -780,7 +782,7 @@ public void sync_beforeUpdate_ShouldNotCallBeforeCreateCallback() {
     }
 
     private static void assertFieldDefinitionsAreEqual(@Nonnull final List<FieldDefinition> oldFields,
-                                             @Nonnull final List<FieldDefinition> newFields) {
+                                                       @Nonnull final List<FieldDefinition> newFields) {
 
         IntStream.range(0, newFields.size())
                 .forEach(index -> {

src/main/java/com/commercetools/sync/services/TypeService.java

@@ -35,8 +35,8 @@ public interface TypeService {
     /**
      * Queries existing {@link Type}'s against set of keys.
      *
-     * @param keys {@link List} of sku values, used in search predicate
-     * @return {@link List} of matching types or empty list when there was not type of key matching to
+     * @param keys {@link Set} of sku values, used in search predicate
+     * @return {@link CompletionStage} of matching types or empty list when there is no type with corresponding
      * {@code keys}.
      */
     @Nonnull
@@ -47,19 +47,19 @@ public interface TypeService {
      * Creates new type from {@code typeDraft}.
      *
      * @param typeDraft draft with data for new type
-     * @return {@link CompletionStage} with created {@link Type} or an exception
+     * @return {@link CompletionStage} with created {@link Type}.
      */
     @Nonnull
     CompletionStage<Type> createType(@Nonnull final TypeDraft typeDraft);
 
     /**
-     * Updates existing product type with {@code updateActions}.
+     * Updates existing type with {@code updateActions}.
      *
      * @param type  type that should be updated
      * @param updateActions {@link List} of actions that should be applied to {@code type}
-     * @return {@link CompletionStage} with updated {@link Type} or an exception
+     * @return {@link CompletionStage} with updated {@link Type}.
      */
     @Nonnull
     CompletionStage<Type> updateType(@Nonnull final Type type,
-                                                   @Nonnull final List<UpdateAction<Type>> updateActions);
+                                     @Nonnull final List<UpdateAction<Type>> updateActions);
 }

src/main/java/com/commercetools/sync/services/impl/TypeServiceImpl.java

@@ -45,7 +45,7 @@ public TypeServiceImpl(@Nonnull final BaseSyncOptions syncOptions) {
     @Override
     public CompletionStage<Optional<String>> fetchCachedTypeId(@Nonnull final String key) {
         if (!isCached) {
-            return fetchAndCache(key);
+            return cacheAndFetch(key);
         }
         return CompletableFuture.completedFuture(Optional.ofNullable(keyToIdCache.get(key)));
     }
@@ -75,7 +75,7 @@ public CompletionStage<Type> updateType(@Nonnull final Type type,
     }
 
     @Nonnull
-    private CompletionStage<Optional<String>> fetchAndCache(@Nonnull final String key) {
+    private CompletionStage<Optional<String>> cacheAndFetch(@Nonnull final String key) {
         final Consumer<List<Type>> typePageConsumer = typesPage ->
             typesPage.forEach(type -> keyToIdCache.put(type.getKey(), type.getId()));
 

src/main/java/com/commercetools/sync/types/TypeSync.java

@@ -65,11 +65,8 @@ public TypeSync(@Nonnull final TypeSyncOptions typeSyncOptions,
      *         processes performed by this sync instance.
      */
     @Override
-    protected CompletionStage<TypeSyncStatistics> process(
-            @Nonnull final List<TypeDraft> typeDrafts) {
-
+    protected CompletionStage<TypeSyncStatistics> process(@Nonnull final List<TypeDraft> typeDrafts) {
         final List<List<TypeDraft>> batches = batchElements(typeDrafts, syncOptions.getBatchSize());
-
         return syncBatches(batches, CompletableFuture.completedFuture(statistics));
     }
 
@@ -90,16 +87,14 @@ protected CompletionStage<TypeSyncStatistics> syncBatches(
      * Fetches existing {@link Type} objects from CTP project that correspond to passed {@code batch}.
      * Having existing types fetched, {@code batch} is compared and synced with fetched objects by
      * {@link TypeSync#syncBatch(List, List)} function. When fetching existing types results in
-     * an empty optional then {@code batch} isn't processed.
+     * an empty updated/created statistics then {@code batch} isn't processed.
      *
      * @param batch batch of drafts that need to be synced
-     * @return {@link CompletionStage} of {@link Void} that indicates method progress.
+     * @return {@link CompletionStage} of {@link TypeSyncStatistics} that indicates method progress.
      */
     @Override
     protected CompletionStage<TypeSyncStatistics> processBatch(@Nonnull final List<TypeDraft> batch) {
-        final List<TypeDraft> validTypeDrafts = batch.stream()
-                                                                   .filter(this::validateDraft)
-                                                                   .collect(toList());
+        final List<TypeDraft> validTypeDrafts = batch.stream().filter(this::validateDraft).collect(toList());
 
         if (validTypeDrafts.isEmpty()) {
             statistics.incrementProcessed(batch.size());
@@ -140,7 +135,7 @@ private boolean validateDraft(@Nullable final TypeDraft draft) {
      * Given a set of type keys, fetches the corresponding types from CTP if they exist.
      *
      * @param keys the keys of the types that are wanted to be fetched.
-     * @return a future which contains the list of types corresponding to the keys.
+     * @return a {@link CompletionStage} which contains the list of types corresponding to the keys.
      */
     private CompletionStage<List<Type>> fetchExistingTypes(@Nonnull final Set<String> keys) {
         return typeService
@@ -188,7 +183,7 @@ private void handleError(@Nonnull final String errorMessage, @Nullable final Thr
      *
      * @param oldTypes old types.
      * @param newTypes drafts that need to be synced.
-     * @return a future which contains an empty result after execution of the update
+     * @return a {@link CompletionStage} which contains an empty result after execution of the update
      */
     private CompletionStage<TypeSyncStatistics> syncBatch(
             @Nonnull final List<Type> oldTypes,
@@ -216,7 +211,7 @@ private CompletionStage<TypeSyncStatistics> syncBatch(
      * is called.
      *
      * @param typeDraft the type draft to create the type from.
-     * @return a future which contains an empty result after execution of the create.
+     * @return a {@link CompletionStage} which contains an empty result after execution of the create.
      */
     private CompletionStage<Void> createType(@Nonnull final TypeDraft typeDraft) {
         return syncOptions.applyBeforeCreateCallBack(typeDraft)
@@ -245,7 +240,7 @@ private CompletionStage<Void> createType(@Nonnull final TypeDraft typeDraft) {
      *
      * @param oldType existing type that could be updated.
      * @param newType draft containing data that could differ from data in {@code oldType}.
-     * @return a future which contains an empty result after execution of the update.
+     * @return a {@link CompletionStage} which contains an empty result after execution of the update.
      */
     @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") // https://github.com/findbugsproject/findbugs/issues/79
     private CompletionStage<Void> updateType(@Nonnull final Type oldType,

src/main/java/com/commercetools/sync/types/utils/FieldDefinitionUpdateActionUtils.java

@@ -22,26 +22,22 @@
 public final class FieldDefinitionUpdateActionUtils {
 
     /**
-     * Compares all the fields of an {@link FieldDefinition} and an {@link FieldDefinition} and returns
+     * Compares all the fields of old {@link FieldDefinition} with new {@link FieldDefinition} and returns
      * a list of {@link UpdateAction}<{@link Type}> as a result. If both the {@link FieldDefinition}
      * and the {@link FieldDefinition} have identical fields, then no update action is needed and hence an
      * empty {@link List} is returned.
      *
-     * @param oldFieldDefinition  the field definition which should be updated.
-     * @param newFieldDefinition the field definition draft where we get the new fields.
+     * @param oldFieldDefinition the old field definition which should be updated.
+     * @param newFieldDefinition the new field definition where we get the new fields.
      * @return A list with the update actions or an empty list if the field definition fields are identical.
      */
     @Nonnull
     public static List<UpdateAction<Type>> buildActions(
         @Nonnull final FieldDefinition oldFieldDefinition,
         @Nonnull final FieldDefinition newFieldDefinition) {
 
-        final List<UpdateAction<Type>> updateActions;
-
-        updateActions = Stream
-            .of(
-                buildChangeLabelUpdateAction(oldFieldDefinition, newFieldDefinition)
-            )
+        final List<UpdateAction<Type>> updateActions = Stream.of(
+                buildChangeLabelUpdateAction(oldFieldDefinition, newFieldDefinition))
             .filter(Optional::isPresent)
             .map(Optional::get)
             .collect(toList());
@@ -86,13 +82,13 @@ private static boolean isLocalizedEnumField(@Nonnull final FieldDefinition field
     }
 
     /**
-     * Compares the {@link LocalizedString} labels of an {@link FieldDefinition} and an
+     * Compares the {@link LocalizedString} labels of old {@link FieldDefinition} with new
      * {@link FieldDefinition} and returns an {@link UpdateAction}&lt;{@link Type}&gt; as a result in
      * an {@link Optional}. If both the {@link FieldDefinition} and the {@link FieldDefinition} have the
      * same label, then no update action is needed and hence an empty {@link Optional} is returned.
      *
-     * @param oldFieldDefinition the field definition which should be updated.
-     * @param newFieldDefinition the field definition draft where we get the new label.
+     * @param oldFieldDefinition the old field definition which should be updated.
+     * @param newFieldDefinition the new field definition draft where we get the new label.
      * @return A filled optional with the update action or an empty optional if the labels are identical.
      */
     @Nonnull

src/main/java/com/commercetools/sync/types/utils/TypeUpdateActionUtils.java

@@ -22,10 +22,9 @@
 public final class TypeUpdateActionUtils {
 
     /**
-     * Compares the {@code name} values of a {@link Type} and a {@link Type}
+     * Compares the {@code name} values of a {@link Type} and a {@link TypeDraft}
      * and returns an {@link Optional} of update action, which would contain the {@code "changeName"}
-     * {@link UpdateAction}. If both {@link Type} and {@link Type} have the same
-     * {@code name} values, then no update action is needed and empty optional will be returned.
+     * {@link UpdateAction} if values are different.
      *
      * @param oldType the type that should be updated.
      * @param newType the type draft which contains the new name.
@@ -43,9 +42,8 @@ public static Optional<UpdateAction<Type>> buildChangeNameAction(
 
     /**
      * Compares the {@link LocalizedString} descriptions of a {@link Type} and a {@link TypeDraft} and
-     * returns an {@link UpdateAction}&lt;{@link Type}&gt; as a result in an {@link Optional}. If both the
-     * {@link Type} and the {@link TypeDraft} have the same description, then no update action is needed and
-     * hence an empty {@link Optional} is returned.
+     * returns an {@link UpdateAction}&lt;{@link Type}&gt; as a result in an {@link Optional}
+     * of update action if values are different.
      *
      * @param oldType the type which should be updated.
      * @param newType the type draft where we get the new description.
@@ -60,10 +58,9 @@ public static Optional<UpdateAction<Type>> buildSetDescriptionUpdateAction(
     }
 
     /**
-     * Compares the fields of a {@link Type} and a {@link TypeDraft} and returns a list of
-     * {@link UpdateAction}&lt;{@link Type}&gt; as a result. If both the {@link Type} and
-     * the {@link TypeDraft} have identical field definitions, then no update action is needed and hence an empty
-     * {@link List} is returned. In case, the new type draft has a list of field definitions in which a
+     * Compares the field definitions of a {@link Type} and a {@link TypeDraft} and returns a list of
+     * {@link UpdateAction}&lt;{@link Type}&gt; as a result in an {@link Optional} of update action
+     * if values are different.  In case, the new type draft has a list of field definitions in which a
      * duplicate name exists, the error callback is triggered and an empty list is returned.
      *
      * @param oldType the type which should be updated.

src/main/java/com/commercetools/sync/types/utils/TypeUpdateEnumActionsUtils.java

@@ -99,7 +99,7 @@ public static <T extends WithKey> Optional<UpdateAction<Type>> buildChangeEnumVa
      * Otherwise, if there are no new enum values, then an empty list is returned.
      *
      * @param fieldDefinitionName     the field definition name whose enum values belong to.
-     * @param oldEnumValues           the list of olf enum values.
+     * @param oldEnumValues           the list of old enum values.
      * @param newEnumValues           the list of new enum values.
      * @param addEnumCallback         the function that is called in order to add the new enum instance
      * @param <T>                     the enum type of the element to add.