Skip to content

Commit

Permalink
disallow "*/key" on all levels except root level;
Browse files Browse the repository at this point in the history 
adapt merging of thing in MetadataFromCommand;
adapt unit tests;

Signed-off-by: Stefan Maute <stefan.maute@bosch.io>
  • Loading branch information
Stefan Maute committed Jun 29, 2022
1 parent 8d2210f commit 3aa1446
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 569 deletions.
20 changes: 0 additions & 20 deletions ...src/main/java/org/eclipse/ditto/base/model/headers/metadata/DefaultMetadataHeaderKey.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
import static org.eclipse.ditto.base.model.common.ConditionChecker.argumentNotEmpty;
import static org.eclipse.ditto.base.model.common.ConditionChecker.checkNotNull;

import java.text.MessageFormat;
import java.util.Comparator;
import java.util.NoSuchElementException;
import java.util.Objects;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -82,11 +80,6 @@ private void validate() {
if (path.isEmpty()) {
throw new IllegalArgumentException("The path of a metadata header key must not be empty!");
}
if (appliesToAllLeaves() && 2 != path.getLevelCount()) {
throw new IllegalArgumentException(MessageFormat.format(
"A wildcard path of a metadata header key must have exactly two levels but it had <{0}>!",
path.getLevelCount()));
}
}

@Override
Expand All @@ -98,19 +91,6 @@ public boolean appliesToAllLeaves() {

@Override
public JsonPointer getPath() {
final JsonPointer result;
if (appliesToAllLeaves()) {

// The sub-pointer consists only of the leaf in this case. This is guaranteed by validation.
result = path.getSubPointer(1).orElseThrow(NoSuchElementException::new);
} else {
result = path;
}
return result;
}

@Override
public JsonPointer getOriginalPath() {
return path;
}

Expand Down
7 changes: 0 additions & 7 deletions .../model/src/main/java/org/eclipse/ditto/base/model/headers/metadata/MetadataHeaderKey.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,6 @@ static MetadataHeaderKey of(final JsonPointer path) {
*/
JsonPointer getPath();

/**
* Returns the original path of this key.
*
* @return the path.
*/
JsonPointer getOriginalPath();

/**
* Returns this key as string as it would appear in DittoHeaders.
*
Expand Down
22 changes: 3 additions & 19 deletions ...test/java/org/eclipse/ditto/base/model/headers/metadata/DefaultMetadataHeaderKeyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
import static org.mutabilitydetector.unittesting.MutabilityAssert.assertInstancesOf;
import static org.mutabilitydetector.unittesting.MutabilityMatchers.areImmutable;

import org.assertj.core.api.AutoCloseableSoftAssertions;
import org.eclipse.ditto.json.JsonFactory;
import org.eclipse.ditto.json.JsonKey;
import org.eclipse.ditto.json.JsonPointer;
import org.junit.Test;

import nl.jqno.equalsverifier.EqualsVerifier;
import org.assertj.core.api.AutoCloseableSoftAssertions;
import org.junit.Test;

/**
* Unit tests for {@link org.eclipse.ditto.base.model.headers.metadata.DefaultMetadataHeaderKey}.
Expand Down Expand Up @@ -62,22 +62,6 @@ public void tryToGetInstanceWithEmptyPath() {
.withNoCause();
}

@Test
public void wildcardPathHasTooFewLevels() {
assertThatIllegalArgumentException()
.isThrownBy(() -> DefaultMetadataHeaderKey.of(JsonPointer.of("/*")))
.withMessage("A wildcard path of a metadata header key must have exactly two levels but it had <1>!")
.withNoCause();
}

@Test
public void wildcardPathHasTooManyLevels() {
assertThatIllegalArgumentException()
.isThrownBy(() -> DefaultMetadataHeaderKey.of(JsonPointer.of("/*/foo/meta")))
.withMessage("A wildcard path of a metadata header key must have exactly two levels but it had <3>!")
.withNoCause();
}

@Test
public void tryToParseNullString() {
assertThatNullPointerException()
Expand Down Expand Up @@ -117,7 +101,7 @@ public void getPathOfWildcardKey() {
final JsonPointer path = JsonFactory.newPointer(DefaultMetadataHeaderKey.HIERARCHY_WILDCARD, leaf);
final MetadataHeaderKey underTest = DefaultMetadataHeaderKey.of(path);

assertThat((CharSequence) underTest.getPath()).isEqualTo(leaf.asPointer());
assertThat((CharSequence) underTest.getPath()).isEqualTo(path);
}

@Test
Expand Down
334 changes: 109 additions & 225 deletions ...things/service/persistence/actors/strategies/commands/MetadataFieldsWildcardResolver.java
View file

Large diffs are not rendered by default.

108 changes: 37 additions & 71 deletions ...ipse/ditto/things/service/persistence/actors/strategies/commands/MetadataFromCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,15 @@
import org.eclipse.ditto.base.model.headers.DittoHeaders;
import org.eclipse.ditto.base.model.headers.metadata.MetadataHeader;
import org.eclipse.ditto.base.model.headers.metadata.MetadataHeaderKey;
import org.eclipse.ditto.base.model.json.FieldType;
import org.eclipse.ditto.base.model.signals.WithOptionalEntity;
import org.eclipse.ditto.base.model.signals.commands.Command;
import org.eclipse.ditto.json.JsonKey;
import org.eclipse.ditto.json.JsonFactory;
import org.eclipse.ditto.json.JsonObject;
import org.eclipse.ditto.json.JsonPointer;
import org.eclipse.ditto.json.JsonValue;
import org.eclipse.ditto.things.model.Thing;
import org.eclipse.ditto.things.model.ThingsModelFactory;
import org.eclipse.ditto.things.model.signals.commands.modify.CreateThing;
import org.eclipse.ditto.things.model.signals.commands.modify.MergeThing;

/**
* Creates or extends/modifies Metadata of an entity based on {@link MetadataHeader}s of a {@link Command}'s
Expand Down Expand Up @@ -79,16 +78,25 @@ static MetadataFromCommand of(final Command<?> command,
@Nullable final Thing existingThing,
@Nullable final Metadata existingMetadata) {

final Thing existingOrEmptyThing = Objects.requireNonNullElseGet(existingThing, () -> Thing.newBuilder().build());
final Thing existingOrEmptyThing =
Objects.requireNonNullElseGet(existingThing, () -> Thing.newBuilder().build());

if (command instanceof WithOptionalEntity withOptionalEntity) {
final var mergedThing = withOptionalEntity.getEntity()
.map(entity -> {
final var resourcePath = command.getResourcePath();
if (resourcePath.isEmpty() && entity.isObject()) {
return ThingsModelFactory.newThing(entity.asObject());
final JsonObject mergedThingJson =
JsonFactory.newObject(entity.asObject(), existingOrEmptyThing.toJson());
return ThingsModelFactory.newThing(mergedThingJson);
} else if (command instanceof MergeThing && entity.isObject()) {
final JsonObject mergedJson =
JsonFactory.newObject(entity.asObject(),
existingOrEmptyThing.toJson().get(resourcePath));
return ThingsModelFactory.newThing(existingOrEmptyThing.toJson().setValue(resourcePath, mergedJson));
} else {
return ThingsModelFactory.newThing(existingOrEmptyThing.toJson().setValue(resourcePath, entity));
return ThingsModelFactory.newThing(
existingOrEmptyThing.toJson().setValue(resourcePath, entity));
}
})
.orElse(existingOrEmptyThing);
Expand Down Expand Up @@ -118,30 +126,21 @@ static MetadataFromCommand of(final Command<?> command,
@Nullable
public Metadata get() {
return getInlineMetadata().orElseGet(() -> {
final JsonValue commandEntity;
if (command instanceof WithOptionalEntity withOptionalEntity) {
commandEntity = withOptionalEntity.getEntity(command.getDittoHeaders()
.getSchemaVersion()
.orElse(command.getLatestSchemaVersion()))
.orElse(null);
} else {
commandEntity = null;
}

final SortedSet<MetadataHeader> metadataHeaders = getMetadataHeadersToPut();
if (null != commandEntity && !metadataHeaders.isEmpty()) {
// TODO this should probably work without a command entity
final Optional<JsonValue> optionalJsonValue = mergedThing.toJson().getValue(command.getResourcePath());
if (!metadataHeaders.isEmpty() && optionalJsonValue.isPresent()) {
final var expandedMetadataHeaders = metadataHeaders.stream()
.flatMap(this::expandWildcards)
.collect(Collectors.toCollection(LinkedHashSet::new));
return buildMetadata(commandEntity, expandedMetadataHeaders);
return buildMetadata(optionalJsonValue.get(), expandedMetadataHeaders);
}
return existingMetadata;
});
}

private Optional<Metadata> getInlineMetadata() {
return command instanceof CreateThing createThing ? createThing.getThing()
.getMetadata() : Optional.empty();
return command instanceof CreateThing createThing ? createThing.getThing().getMetadata() : Optional.empty();
}

private SortedSet<MetadataHeader> getMetadataHeadersToPut() {
Expand All @@ -151,8 +150,10 @@ private SortedSet<MetadataHeader> getMetadataHeadersToPut() {

private Stream<MetadataHeader> expandWildcards(final MetadataHeader metadataHeader) {
if (containsWildcard(metadataHeader)) {
MetadataWildcardValidator.validateMetadataWildcard(command.getResourcePath(),
metadataHeader.getKey().toString(), DittoHeaderDefinition.PUT_METADATA.getKey());
final var jsonPointers = MetadataFieldsWildcardResolver.resolve(command, mergedThing,
metadataHeader.getKey().getOriginalPath(), DittoHeaderDefinition.PUT_METADATA.getKey());
metadataHeader.getKey().getPath(), DittoHeaderDefinition.PUT_METADATA.getKey());
return jsonPointers.stream()
.map(MetadataHeaderKey::of)
.map(metadataHeaderKey -> MetadataHeader.of(metadataHeaderKey, metadataHeader.getValue()));
Expand All @@ -174,68 +175,33 @@ private Metadata buildMetadata(final JsonValue entity, final Set<MetadataHeader>
final MetadataHeaderKey metadataHeaderKey = metadataHeader.getKey();
final JsonValue metadataHeaderValue = metadataHeader.getValue();

if (metadataHeaderKey.appliesToAllLeaves()) {
addMetadataToLeaf(JsonPointer.empty(), metadataHeader, metadataBuilder, entity);
} else {
if (entity.isObject()) {
final var field = entity.asObject()
.getField(metadataHeaderKey.getPath());
final var metadataHeaderKeyLeaf = metadataHeaderKey.getPath()
.cutLeaf();
if (field.isPresent()) {
if (field.get()
.getValue()
.isObject()) {
metadataBuilder.set(metadataHeaderKey.getPath(), metadataHeaderValue);
}
} else if (entity.asObject()
.contains(metadataHeaderKeyLeaf)) {
final var optionalLeafValue = entity.asObject()
.getValue(metadataHeaderKeyLeaf);
if (optionalLeafValue.isEmpty() || optionalLeafValue.get()
.isObject() || metadataHeaderKey.getPath()
.getLevelCount() > 1) {
metadataBuilder.set(metadataHeaderKey.getPath(), metadataHeaderValue);
}
} else if (metadataHeaderKey.getPath()
.getLevelCount() == 1) {
if (entity.isObject()) {
final var field = entity.asObject().getField(metadataHeaderKey.getPath());
final var metadataHeaderKeyLeaf = metadataHeaderKey.getPath().cutLeaf();
if (field.isPresent()) {
if (field.get().getValue().isObject()) {
metadataBuilder.set(metadataHeaderKey.getPath(), metadataHeaderValue);
}
} else if (entity.asObject().contains(metadataHeaderKeyLeaf)) {
final var optionalLeafValue = entity.asObject().getValue(metadataHeaderKeyLeaf);
if (optionalLeafValue.isEmpty() || optionalLeafValue.get().isObject()
|| metadataHeaderKey.getPath().getLevelCount() > 1) {
metadataBuilder.set(metadataHeaderKey.getPath(), metadataHeaderValue);
}
} else if (metadataHeaderKey.getPath()
.getLevelCount() > 1 || metadataHeaderValue.isObject()) {
} else if (metadataHeaderKey.getPath().getLevelCount() == 1) {
metadataBuilder.set(metadataHeaderKey.getPath(), metadataHeaderValue);
}
} else if (metadataHeaderKey.getPath().getLevelCount() > 1 || metadataHeaderValue.isObject()) {
metadataBuilder.set(metadataHeaderKey.getPath(), metadataHeaderValue);
}
};

metadataHeaders.forEach(addMetadataToBuilder);

return metadataBuilder.build();
}

private MetadataBuilder getMetadataBuilder() {
return null != existingMetadata ? Metadata.newBuilder()
.setAll(existingMetadata) : Metadata.newBuilder();
}

private static void addMetadataToLeaf(final JsonPointer path,
final MetadataHeader metadataHeader,
final MetadataBuilder metadataBuilder,
final JsonValue entity) {

if (entity.isObject()) {
final JsonObject entityObject = entity.asObject();
entityObject.stream()
.filter(field -> !(field.isMarkedAs(FieldType.SPECIAL) || field.isMarkedAs(FieldType.HIDDEN)))
.forEach(jsonField -> {
final JsonKey key = jsonField.getKey();
addMetadataToLeaf(path.append(key.asPointer()), metadataHeader, metadataBuilder,
jsonField.getValue());
});
} else {
final MetadataHeaderKey metadataHeaderKey = metadataHeader.getKey();
metadataBuilder.set(path.append(metadataHeaderKey.getPath()), metadataHeader.getValue());
}
return null != existingMetadata ? Metadata.newBuilder().setAll(existingMetadata) : Metadata.newBuilder();
}

}
24 changes: 0 additions & 24 deletions ...itto/things/service/persistence/actors/strategies/commands/MetadataWildcardValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,7 @@ final class MetadataWildcardValidator {

public static final JsonPointer ROOT_PATH = JsonPointer.of("/");
public static final JsonPointer FEATURES_PATH = Thing.JsonFields.FEATURES.getPointer();
public static final JsonPointer ATTRIBUTES_PATH = Thing.JsonFields.ATTRIBUTES.getPointer();
public static final String FEATURE_PATH = FEATURES_PATH + "/.*";
public static final String FEATURE_PROPERTIES_PATH = FEATURES_PATH + "/.*/properties";
public static final String FEATURE_DESIRED_PROPERTIES_PATH = FEATURES_PATH + "/.*/desiredProperties";
public static final String FEATURE_PROPERTY_PATH = FEATURES_PATH + "/.*/properties/.*";
public static final String FEATURE_DESIRED_PROPERTY_PATH = FEATURES_PATH + "/.*/desiredProperties/.*";

public static final String FEATURE_DEFINITION_PATH = FEATURES_PATH + "/.*/definition/.*";
public static final String FEATURES_SUB_PATH = FEATURES_PATH + "/.*/(properties|desiredProperties)(.*)";

private static final String THING_FEATURES_AND_PROPERTIES_WILDCARD_REGEX =
"/?features/\\*/(properties|desiredProperties)/\\*/(?!\\*).*";
Expand Down Expand Up @@ -82,11 +74,6 @@ public static void validateMetadataWildcard(final JsonPointer resourcePath, fina
validateWildcardExpressionOnFeaturesLevel(metaDataWildcardExpression, headerKey);
} else if (levelCount == 2 && resourcePathAsString.matches(FEATURE_PATH)) {
validateWildcardExpressionOnFeatureLevel(metaDataWildcardExpression, headerKey);
} else if ((levelCount >= 3 && resourcePathAsString.matches(FEATURES_SUB_PATH))
|| resourcePath.equals(ATTRIBUTES_PATH)) {
if (!Pattern.matches(LEAF_WILDCARD_REGEX, metaDataWildcardExpression)) {
throw getDittoHeaderInvalidException(metaDataWildcardExpression, headerKey);
}
} else {
throw getDittoHeaderNotSupportedException(metaDataWildcardExpression, headerKey);
}
Expand Down Expand Up @@ -200,17 +187,6 @@ public static boolean matchesFeaturesWithPropertyOnlyWildcard(final String wildc
return Pattern.matches(FEATURES_WITH_PROPERTIES_ONLY_WILDCARD_REGEX, wildcardExpression);
}

/**
* Matches the passed {@code wildcardExpression} against wildcard regex
* {@code FEATURES_DEFINITION_WILDCARD_REGEX} and returns {@code true} if they match.
*
* @param wildcardExpression the wildcard expression that should be checked.
* @return {@code true} if the wildcardExpression matches the regex and false if not.
*/
public static boolean matchesFeaturesDefinitionWildcard(final String wildcardExpression) {
return Pattern.matches(FEATURES_DEFINITION_WILDCARD_REGEX, wildcardExpression);
}

/**
* Matches the passed {@code wildcardExpression} against wildcard regex {@code FEATURE_PROPERTY_WILDCARD_REGEX}
* and returns {@code true} if they match.
Expand Down
14 changes: 14 additions & 0 deletions ...ditto/things/service/persistence/actors/strategies/events/AbstractThingEventStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
package org.eclipse.ditto.things.service.persistence.actors.strategies.events;

import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
Expand All @@ -22,6 +24,8 @@
import org.eclipse.ditto.base.model.signals.commands.Command;
import org.eclipse.ditto.internal.utils.persistentactors.events.EventStrategy;
import org.eclipse.ditto.json.JsonFactory;
import org.eclipse.ditto.json.JsonKey;
import org.eclipse.ditto.json.JsonObject;
import org.eclipse.ditto.json.JsonPointer;
import org.eclipse.ditto.json.JsonValue;
import org.eclipse.ditto.things.model.Thing;
Expand Down Expand Up @@ -106,6 +110,16 @@ private Metadata deleteMetadataForMergeAndModifiedEvents(final T event, final Me
final Optional<JsonValue> optionalJsonValue = event.getEntity();
if (optionalJsonValue.isEmpty() || optionalJsonValue.get().isNull()) {
return metadataBuilder.remove(event.getResourcePath()).build();
} else if (optionalJsonValue.get().isObject()) {
final JsonObject jsonObject = optionalJsonValue.get().asObject();
final Set<JsonKey> jsonKeysForNullValue = jsonObject.getKeys().stream()
.filter(jsonKey -> jsonObject.getValue(jsonKey).orElseThrow().isNull())
.collect(Collectors.toSet());

jsonKeysForNullValue.forEach(jsonKey ->
metadataBuilder.remove(event.getResourcePath() + "/" + jsonKey.toString()));

return metadataBuilder.build();
}
} else if (event instanceof ThingModifiedEvent && event.getCommandCategory().equals(Command.Category.MODIFY)) {
final Optional<JsonValue> optionalJsonValue = event.getEntity();
Expand Down

0 comments on commit 3aa1446

Please sign in to comment.