review:

minor code format changes and readability improvements; Signed-off-by: Stefan Maute <stefan.maute@bosch.io>
eclipse-ditto · Jan 11, 2022 · b672b83 · b672b83
1 parent ae7e8ae
commit b672b83
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 28 deletions.
diff --git a/...ava/org/eclipse/ditto/internal/models/signalenrichment/CachingSignalEnrichmentFacade.java b/...ava/org/eclipse/ditto/internal/models/signalenrichment/CachingSignalEnrichmentFacade.java
@@ -49,34 +49,43 @@ public interface CachingSignalEnrichmentFacade extends SignalEnrichmentFacade {
 
     default JsonObject applyJsonFieldSelector(final JsonObject jsonObject,
             @Nullable final JsonFieldSelector fieldSelector) {
+        final JsonObject result;
+
         if (fieldSelector == null) {
-            return jsonObject;
+            result = jsonObject;
         } else {
             final Collection<JsonKey> featureIds = jsonObject.getValue(Thing.JsonFields.FEATURES.getPointer())
                     .filter(JsonValue::isObject)
                     .map(JsonValue::asObject)
                     .map(JsonObject::getKeys)
                     .orElse(Collections.emptyList());
             final JsonFieldSelector jsonPointers = expandFeatureIdWildcard(fieldSelector, featureIds);
-            return jsonObject.get(jsonPointers);
+            result = jsonObject.get(jsonPointers);
         }
+
+        return result;
     }
 
     private static JsonFieldSelector expandFeatureIdWildcard(final JsonFieldSelector fieldSelector,
             final Collection<JsonKey> featureIds) {
 
-        return JsonFactory.newFieldSelector(fieldSelector.getPointers().stream().flatMap(p -> {
-            if (p.getLevelCount() > 1
-                    &&
-                    p.getRoot().map(k -> Thing.JsonFields.FEATURES.getPointer().equals(JsonPointer.of(k))).orElse(false)
-                    && p.get(1).map(k -> JsonKey.of("*").equals(k)).orElse(false)) {
-                return featureIds.stream()
-                        .map(fid -> Thing.JsonFields.FEATURES.getPointer()
-                                .append(JsonPointer.of(fid))
-                                .append(p.getSubPointer(2).orElse(JsonPointer.empty())));
-            } else {
-                return Stream.of(p);
-            }
-        }).collect(Collectors.toList()));
+        final List<JsonPointer> jsonPointerList = fieldSelector.getPointers().stream()
+                .flatMap(jsonPointer -> {
+                    if (jsonPointer.getLevelCount() > 1
+                            && jsonPointer.getRoot()
+                            .map(k -> Thing.JsonFields.FEATURES.getPointer().equals(JsonPointer.of(k)))
+                            .orElse(false)
+                            && jsonPointer.get(1).map(k -> JsonKey.of("*").equals(k)).orElse(false)) {
+                        return featureIds.stream()
+                                .map(fid -> Thing.JsonFields.FEATURES.getPointer()
+                                        .append(JsonPointer.of(fid))
+                                        .append(jsonPointer.getSubPointer(2).orElse(JsonPointer.empty())));
+                    } else {
+                        return Stream.of(jsonPointer);
+                    }
+                }).collect(Collectors.toList());
+
+        return JsonFactory.newFieldSelector(jsonPointerList);
     }
+
 }
diff --git a/...rg/eclipse/ditto/internal/models/signalenrichment/DittoCachingSignalEnrichmentFacade.java b/...rg/eclipse/ditto/internal/models/signalenrichment/DittoCachingSignalEnrichmentFacade.java
@@ -102,7 +102,9 @@ public CompletionStage<JsonObject> retrieveThing(final ThingId thingId, final Li
             extraFieldsCache.invalidate(cacheKey);
             return doCacheLookup(cacheKey, dittoHeaders);
         } else {
-            final var cachingParameters = new CachingParameters(null, events, false, minAcceptableSeqNr);
+            final var cachingParameters =
+                    new CachingParameters(null, events, false, minAcceptableSeqNr);
+
             return doRetrievePartialThing(thingId, dittoHeaders, cachingParameters);
         }
     }
@@ -117,7 +119,9 @@ public CompletionStage<JsonObject> retrievePartialThing(final ThingId thingId,
                         List.of((ThingEvent<?>) concernedSignal) : List.of();
 
         // as second step only return what was originally requested as fields:
-        final var cachingParameters = new CachingParameters(jsonFieldSelector, thingEvents, true, 0);
+        final var cachingParameters =
+                new CachingParameters(jsonFieldSelector, thingEvents, true, 0);
+
         return doRetrievePartialThing(thingId, dittoHeaders, cachingParameters)
                 .thenApply(jsonObject -> applyJsonFieldSelector(jsonObject, jsonFieldSelector));
     }
@@ -146,7 +150,9 @@ public CompletionStage<JsonObject> retrievePartialThing(final EntityId thingId,
                 .collect(Collectors.toList());
 
         // as second step only return what was originally requested as fields:
-        final var cachingParameters = new CachingParameters(jsonFieldSelector, thingEvents, true, minAcceptableSeqNr);
+        final var cachingParameters =
+                new CachingParameters(jsonFieldSelector, thingEvents, true, minAcceptableSeqNr);
+
         return doRetrievePartialThing(thingId, dittoHeaders, cachingParameters)
                 .thenApply(jsonObject -> applyJsonFieldSelector(jsonObject, jsonFieldSelector));
     }
@@ -173,6 +179,7 @@ private CompletionStage<JsonObject> doRetrievePartialThing(final EntityId thingI
     private static JsonFieldSelector enhanceFieldSelectorWithRevision(
             @Nullable final Iterable<JsonPointer> fieldSelector) {
         final JsonFieldSelector result;
+
         if (fieldSelector == null) {
             result = null;
         } else {
@@ -181,6 +188,7 @@ private static JsonFieldSelector enhanceFieldSelectorWithRevision(
                     .addFieldDefinition(Thing.JsonFields.REVISION) // additionally always select the revision
                     .build();
         }
+
         return result;
     }
 
@@ -258,6 +266,7 @@ private static Optional<Integer> findLastThingDeletedOrCreated(final List<ThingE
                 return Optional.of(i);
             }
         }
+
         return Optional.empty();
     }
 
@@ -271,8 +280,8 @@ private static DittoHeaders getLastDittoHeaders(final List<? extends Signal<?>>
 
     private CompletableFuture<JsonObject> doCacheLookup(final SignalEnrichmentCacheKey cacheKey,
             final DittoHeaders dittoHeaders) {
-
         LOGGER.withCorrelationId(dittoHeaders).debug("Looking up cache entry for <{}>", cacheKey);
+
         return extraFieldsCache.get(cacheKey)
                 .thenApply(optionalJsonObject -> optionalJsonObject.orElseGet(JsonObject::empty));
     }
@@ -310,6 +319,7 @@ private CompletionStage<JsonObject> doSmartUpdateCachedObject(final SignalEnrich
             extraFieldsCache.invalidate(cacheKey);
             result = doCacheLookup(cacheKey, dittoHeaders);
         }
+
         return result;
     }
 
@@ -354,13 +364,15 @@ private CompletionStage<JsonObject> handleNextExpectedThingEvents(final SignalEn
         final var enhancedJsonObject = enhanceJsonObject(jsonObject, concernedSignals, enhancedFieldSelector);
         // update local cache with enhanced object:
         extraFieldsCache.put(cacheKey, enhancedJsonObject);
+
         return CompletableFuture.completedFuture(enhancedJsonObject);
     }
 
     private static JsonObject getMergeJsonObject(final JsonValue jsonObject, final ThingEvent<?> thingEvent) {
         final var thingMerged = (ThingMerged) thingEvent;
         final JsonValue mergedValue = thingMerged.getValue();
         final JsonObject mergePatch = JsonFactory.newObject(thingMerged.getResourcePath(), mergedValue);
+
         return JsonFactory.mergeJsonValues(mergePatch, jsonObject).asObject();
     }
 
@@ -375,6 +387,7 @@ private static JsonObject getDeleteJsonObject(final JsonObject jsonObject, final
         } else {
             result = jsonObject.remove(resourcePath);
         }
+
         return result;
     }
 
@@ -389,6 +402,7 @@ private static JsonObject getDefaultJsonObject(final JsonObject jsonObject, fina
                     .set(resourcePath.toString(), entity)
             );
         }
+
         return jsonObjectBuilder.build();
     }
 

diff --git a/.../service/persistence/actors/strategies/commands/AbstractRetrieveThingCommandStrategy.java b/.../service/persistence/actors/strategies/commands/AbstractRetrieveThingCommandStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 Contributors to the Eclipse Foundation
+ * Copyright (c) 2022 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -59,14 +59,15 @@ static JsonFieldSelector expandFeatureIdWildcard(final JsonFieldSelector fieldSe
                     return Stream.of(p);
                 }
             }).collect(Collectors.toList());
+
             return JsonFactory.newFieldSelector(expanded);
         }
 
     }
 
     static JsonPointer replaceLevel(final JsonPointer source, final int level, final JsonPointer replacement) {
 
-        if (source.getLevelCount() <= level) {
+        if (source.getLevelCount() <= level || level < 0) {
             return source;
         } else {
             if (level == 0) {
@@ -81,4 +82,5 @@ static JsonPointer replaceLevel(final JsonPointer source, final int level, final
             }
         }
     }
+
 }
diff --git a/...vice/persistence/actors/strategies/commands/AbstractRetrieveThingCommandStrategyTest.java b/...vice/persistence/actors/strategies/commands/AbstractRetrieveThingCommandStrategyTest.java
@@ -62,12 +62,15 @@ public void testReplaceLevel() {
         testReplaceLevel("a/b/c", 0, "a", "a/b/c");
         testReplaceLevel("a/b/*", 2, "c", "a/b/c");
         testReplaceLevel("a/b/c", 3, "x", "a/b/c");
+        testReplaceLevel("a/b/c/d", -1, "x", "a/b/c/d");
     }
 
     private void testReplaceLevel(final String original, final int level, final String replacement,
             final String expected) {
+
         final JsonPointer actual = AbstractRetrieveThingCommandStrategy.replaceLevel(JsonPointer.of(original), level,
                 JsonPointer.of(replacement));
+
         assertThat((Object) actual).isEqualTo(JsonPointer.of(expected));
     }
 }
diff --git a/...itto/things/service/persistence/actors/strategies/commands/RetrieveThingStrategyTest.java b/...itto/things/service/persistence/actors/strategies/commands/RetrieveThingStrategyTest.java
@@ -114,7 +114,6 @@ public void unhandledReturnsThingNotAccessibleException() {
         assertUnhandledResult(underTest, THING_V2, command, expectedException);
     }
 
-
     @Test
     public void retrieveThingWithSelectedFields() {
         final CommandStrategy.Context<ThingId> context = getDefaultContext();
@@ -134,7 +133,7 @@ public void retrieveThingWithSelectedFields() {
     @Test
     public void retrieveThingWithSelectedFieldsWithFeatureWildcard() {
         final CommandStrategy.Context<ThingId> context = getDefaultContext();
-        final JsonFieldSelector fieldSelector = JsonFactory.newFieldSelector("/attribute/location",
+        final JsonFieldSelector fieldSelector = JsonFactory.newFieldSelector("/attributes/location",
                 "/features/*/properties/target_year_1");
         final DittoHeaders dittoHeaders = DittoHeaders.newBuilder()
                 .schemaVersion(JsonSchemaVersion.V_2)
@@ -144,8 +143,8 @@ public void retrieveThingWithSelectedFieldsWithFeatureWildcard() {
         THING_V2.getFeatures().orElseThrow().forEach(featuresBuilder::set);
         featuresBuilder.set(ThingsModelFactory.newFeature("f1",
                 ThingsModelFactory.newFeaturePropertiesBuilder().set("target_year_1", 2022).build()));
-        featuresBuilder.set(ThingsModelFactory.newFeature("f2", ThingsModelFactory.newFeaturePropertiesBuilder().set(
-                "connected", false).build()));
+        featuresBuilder.set(ThingsModelFactory.newFeature("f2",
+                ThingsModelFactory.newFeaturePropertiesBuilder().set("connected", false).build()));
         final Thing thing = THING_V2.toBuilder()
                 .setFeatures(featuresBuilder.build())
                 .build();
@@ -154,14 +153,16 @@ public void retrieveThingWithSelectedFieldsWithFeatureWildcard() {
                 .withSelectedFields(fieldSelector)
                 .build();
 
-        final JsonFieldSelector expandedFieldSelector = JsonFactory.newFieldSelector("/attribute/location",
+        final JsonFieldSelector expandedFieldSelector = JsonFactory.newFieldSelector("/attributes/location",
                 "/features/" + TestConstants.Feature.FLUX_CAPACITOR_ID + "/properties/target_year_1",
                 "/features/f1/properties/target_year_1",
                 "/features/f2/properties/target_year_1");
+
         assertQueryResult(underTest, thing, command, response -> {
             assertThat(response).isInstanceOf(RetrieveThingResponse.class);
             final RetrieveThingResponse retrieveThingResponse = (RetrieveThingResponse) response;
             assertThat(retrieveThingResponse.getEntity()).isEqualTo(thing.toJson(expandedFieldSelector));
         });
     }
+
 }
diff --git a/.../things/service/persistence/actors/strategies/commands/SudoRetrieveThingStrategyTest.java b/.../things/service/persistence/actors/strategies/commands/SudoRetrieveThingStrategyTest.java
@@ -132,9 +132,9 @@ public void retrieveThingWithSelectedFields() {
     @Test
     public void retrieveThingWithSelectedFieldsWithFeatureWildcard() {
         final CommandStrategy.Context<ThingId> context = getDefaultContext();
-        final JsonFieldSelector fieldSelector = JsonFactory.newFieldSelector("/attribute/location",
+        final JsonFieldSelector fieldSelector = JsonFactory.newFieldSelector("/attributes/location",
                 "/features/*/properties/target_year_1");
-        final JsonFieldSelector expandedFieldSelector = JsonFactory.newFieldSelector("/attribute/location",
+        final JsonFieldSelector expandedFieldSelector = JsonFactory.newFieldSelector("/attributes/location",
                 "/features/" + TestConstants.Feature.FLUX_CAPACITOR_ID + "/properties/target_year_1",
                 "/features/f1/properties/target_year_1",
                 "/features/f2/properties/target_year_1");