From bae69bc58ff2024a58af30e72df7654685163690 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Mon, 15 Mar 2021 17:01:18 +0100 Subject: [PATCH] Introduce an interface for Property. [improvement] This change brings symmetry to the three things that make up a graph - Nodes - Relationships - Properties This change spares us from a further ABI incompatible change after 2021.1.0. --- .../core/AbstractPropertyContainer.java | 4 +- .../java/org/neo4j/cypherdsl/core/Cypher.java | 4 +- .../org/neo4j/cypherdsl/core/Expression.java | 2 +- .../cypherdsl/core/InternalPropertyImpl.java | 149 ++++++++++++++++++ .../org/neo4j/cypherdsl/core/Property.java | 127 ++------------- .../org/neo4j/cypherdsl/core/FunctionsIT.java | 2 +- ...est.java => InternalPropertyImplTest.java} | 8 +- 7 files changed, 171 insertions(+), 125 deletions(-) create mode 100644 neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/InternalPropertyImpl.java rename neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/{PropertyTest.java => InternalPropertyImplTest.java} (74%) diff --git a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/AbstractPropertyContainer.java b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/AbstractPropertyContainer.java index ce954d9c36..d52e3b1750 100644 --- a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/AbstractPropertyContainer.java +++ b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/AbstractPropertyContainer.java @@ -40,12 +40,12 @@ public final Property property(String name) { @Override public final Property property(String... names) { - return Property.create(this, names); + return InternalPropertyImpl.create(this, names); } @Override public final Property property(Expression lookup) { - return Property.create(this, lookup); + return InternalPropertyImpl.create(this, lookup); } @Override diff --git a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Cypher.java b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Cypher.java index 09100d0e97..aedd8cb1e3 100644 --- a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Cypher.java +++ b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Cypher.java @@ -140,7 +140,7 @@ public static Property property(String containerName, String... names) { * @return A new property. */ public static Property property(Expression expression, String... names) { - return Property.create(expression, names); + return InternalPropertyImpl.create(expression, names); } /** @@ -166,7 +166,7 @@ public static Property property(String containerName, Expression lookup) { * @since 2021.0.0 */ public static Property property(Expression expression, Expression lookup) { - return Property.create(expression, lookup); + return InternalPropertyImpl.create(expression, lookup); } /** diff --git a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Expression.java b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Expression.java index 1335bf2781..dbfddb4504 100644 --- a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Expression.java +++ b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Expression.java @@ -353,6 +353,6 @@ default SortItem ascending() { */ default Property property(String... names) { - return Property.create(this, names); + return InternalPropertyImpl.create(this, names); } } diff --git a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/InternalPropertyImpl.java b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/InternalPropertyImpl.java new file mode 100644 index 0000000000..621e200904 --- /dev/null +++ b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/InternalPropertyImpl.java @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2019-2021 "Neo4j," + * Neo4j Sweden AB [https://neo4j.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.cypherdsl.core; + +import static org.apiguardian.api.API.Status.INTERNAL; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.apiguardian.api.API; +import org.neo4j.cypherdsl.core.support.Visitor; +import org.neo4j.cypherdsl.core.utils.Assertions; + +@API(status = INTERNAL, since = "2021.1.0") +final class InternalPropertyImpl implements Property { + + static Property create(Named parentContainer, String... names) { + + SymbolicName requiredSymbolicName = extractRequiredSymbolicName(parentContainer); + return new InternalPropertyImpl(Optional.of(parentContainer), requiredSymbolicName, createListOfChainedNames(names), null); + } + + static Property create(Expression containerReference, String... names) { + + Assertions.notNull(containerReference, "The property container is required."); + return new InternalPropertyImpl(Optional.empty(), containerReference, createListOfChainedNames(names), null); + } + + static Property create(Named parentContainer, Expression lookup) { + + SymbolicName requiredSymbolicName = extractRequiredSymbolicName(parentContainer); + return new InternalPropertyImpl(Optional.of(parentContainer), requiredSymbolicName, + Collections.singletonList(PropertyLookup.forExpression(lookup)), null); + } + + static Property create(Expression containerReference, Expression lookup) { + + return new InternalPropertyImpl(Optional.empty(), containerReference, + Collections.singletonList(PropertyLookup.forExpression(lookup)), null); + } + + /** + * The reference to the container itself is optional. + */ + private final Named container; + + /** + * The expression pointing to the {@link #container} above is not. + */ + private final Expression containerReference; + + /** + * The name of this property. + */ + private final List names; + + /** + * An optional, external (as in external to the graph) reference. + */ + private final String externalReference; + + InternalPropertyImpl(Optional container, Expression containerReference, List names, + String externalReference) { + + this.container = container.orElse(null); + this.containerReference = containerReference; + this.names = names; + this.externalReference = externalReference; + } + + @Override + public List getNames() { + return names; + } + + @Override + public Named getContainer() { + return container; + } + + @Override + public String getName() { + return externalReference != null ? + this.externalReference : + this.names.stream().map(PropertyLookup::getPropertyKeyName) + .map(SymbolicName::getValue) + .collect(Collectors.joining(".")); + } + + @Override + public Property referencedAs(String newReference) { + + return new InternalPropertyImpl(Optional.ofNullable(this.container), containerReference, this.names, + newReference); + } + + @Override + public Operation to(Expression expression) { + return Operations.set(this, expression); + } + + @Override + public void accept(Visitor visitor) { + visitor.enter(this); + this.containerReference.accept(visitor); + this.names.forEach(name -> name.accept(visitor)); + visitor.leave(this); + } + + private static List createListOfChainedNames(String... names) { + + Assertions.notEmpty(names, "The properties name is required."); + + if (names.length == 1) { + return Collections.singletonList(PropertyLookup.forName(names[0])); + } else { + return Arrays.stream(names).map(PropertyLookup::forName) + .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + } + } + + private static SymbolicName extractRequiredSymbolicName(Named parentContainer) { + try { + return parentContainer.getRequiredSymbolicName(); + } catch (IllegalStateException e) { + throw new IllegalArgumentException( + "A property derived from a node or a relationship needs a parent with a symbolic name."); + } + } +} diff --git a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Property.java b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Property.java index 4c59a256b5..70c54ae703 100644 --- a/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Property.java +++ b/neo4j-cypher-dsl/src/main/java/org/neo4j/cypherdsl/core/Property.java @@ -19,131 +19,47 @@ package org.neo4j.cypherdsl.core; import static org.apiguardian.api.API.Status.EXPERIMENTAL; -import static org.apiguardian.api.API.Status.INTERNAL; -import java.util.Arrays; -import java.util.Collections; import java.util.List; -import java.util.Optional; -import java.util.stream.Collectors; import org.apiguardian.api.API; -import org.neo4j.cypherdsl.core.support.Visitor; -import org.neo4j.cypherdsl.core.utils.Assertions; /** * A property. A property might belong to a container such as a {@link Node} or {@link Relationship}, but it's not uncommon * to extract single properties from maps or from various datatypes such as a duration returned from stored procedures. * The container can be retrieved via {@link #getContainer()} in case the property belongs to a node or relationship. *

- * A property has always a reference to the name of the object it was extracted from (see {@link #containerReference}. + * A property has always a reference to the name of the object it was extracted from. * * @author Michael J. Simons * @since 1.0 */ @API(status = EXPERIMENTAL, since = "1.0") -public final class Property implements Expression { - - static Property create(Named parentContainer, String... names) { - - SymbolicName requiredSymbolicName = extractRequiredSymbolicName(parentContainer); - return new Property(Optional.of(parentContainer), requiredSymbolicName, createListOfChainedNames(names), null); - } - - static Property create(Expression containerReference, String... names) { - - Assertions.notNull(containerReference, "The property container is required."); - return new Property(Optional.empty(), containerReference, createListOfChainedNames(names), null); - } - - static Property create(Named parentContainer, Expression lookup) { - - SymbolicName requiredSymbolicName = extractRequiredSymbolicName(parentContainer); - return new Property(Optional.of(parentContainer), requiredSymbolicName, - Collections.singletonList(PropertyLookup.forExpression(lookup)), null); - } - - static Property create(Expression containerReference, Expression lookup) { - - return new Property(Optional.empty(), containerReference, - Collections.singletonList(PropertyLookup.forExpression(lookup)), null); - } - - private static List createListOfChainedNames(String... names) { - - Assertions.notEmpty(names, "The properties name is required."); - - if (names.length == 1) { - return Collections.singletonList(PropertyLookup.forName(names[0])); - } else { - return Arrays.stream(names).map(PropertyLookup::forName) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); - } - } - - /** - * The reference to the container itself is optional. - */ - private final Named container; +public interface Property extends Expression { /** - * The expression pointing to the {@link #container} above is not. - */ - private final Expression containerReference; - - /** - * The name of this property. - */ - private final List names; - - /** - * An optional, external (as in external to the graph) reference. + * Returns the concatenated names of the property or the external reference (See {@link #referencedAs(String)}) if set. + * + * @return A name to reference the property under in an external application */ - private final String externalReference; - - Property(Optional container, Expression containerReference, List names, String externalReference) { - - this.container = container.orElse(null); - this.containerReference = containerReference; - this.names = names; - this.externalReference = externalReference; - } + @API(status = EXPERIMENTAL, since = "2021.1.0") + String getName(); /** * @return The actual property being looked up. The order matters, so this will return a list, not a collection. */ - @API(status = INTERNAL) - List getNames() { - return names; - } + List getNames(); - @API(status = INTERNAL) - Named getContainer() { - return container; - } - - /** - * Returns the concatenated names of the property or the {@link #externalReference} (See {@link #referencedAs(String)}) if set. - * - * @return A name to reference the property under in an external application - */ - @API(status = EXPERIMENTAL, since = "2021.1.0") - public String getName() { - return externalReference != null ? this.externalReference : this.names.stream().map(PropertyLookup::getPropertyKeyName) - .map(SymbolicName::getValue) - .collect(Collectors.joining(".")); - } + Named getContainer(); /** * Creates a new property with an external reference. + * * @param newReference An arbitrary, external reference * @return A new property */ @API(status = EXPERIMENTAL, since = "2021.1.0") - public Property referencedAs(String newReference) { - - return new Property(Optional.ofNullable(this.container), containerReference, this.names, newReference); - } + Property referencedAs(String newReference); /** * Creates an {@link Operation} setting this property to a new value. The property does not track the operations @@ -152,24 +68,5 @@ public Property referencedAs(String newReference) { * @param expression expression describing the new value * @return A new operation. */ - public Operation to(Expression expression) { - return Operations.set(this, expression); - } - - @Override - public void accept(Visitor visitor) { - visitor.enter(this); - this.containerReference.accept(visitor); - this.names.forEach(name -> name.accept(visitor)); - visitor.leave(this); - } - - private static SymbolicName extractRequiredSymbolicName(Named parentContainer) { - try { - return parentContainer.getRequiredSymbolicName(); - } catch (IllegalStateException e) { - throw new IllegalArgumentException( - "A property derived from a node or a relationship needs a parent with a symbolic name."); - } - } + Operation to(Expression expression); } diff --git a/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/FunctionsIT.java b/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/FunctionsIT.java index 86c4845bb0..f99427dcf9 100644 --- a/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/FunctionsIT.java +++ b/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/FunctionsIT.java @@ -96,7 +96,7 @@ void manual() { .returning( Functions.reduce(n) .in(Functions.nodes(p)) - .map(totalAge.add(Property.create(n, "age"))) + .map(totalAge.add(Cypher.property(n, "age"))) .accumulateOn(totalAge) .withInitialValueOf(Cypher.literalOf(0)).as("reduction") ).build(); diff --git a/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/PropertyTest.java b/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/InternalPropertyImplTest.java similarity index 74% rename from neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/PropertyTest.java rename to neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/InternalPropertyImplTest.java index 79c9da676e..471a26d101 100644 --- a/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/PropertyTest.java +++ b/neo4j-cypher-dsl/src/test/java/org/neo4j/cypherdsl/core/InternalPropertyImplTest.java @@ -25,24 +25,24 @@ /** * @author Michael J. Simons */ -class PropertyTest { +class InternalPropertyImplTest { @Test void simplePropertyGetNameShouldWork() { - assertThat(Property.create(SymbolicName.unresolved(), "simple").getName()).isEqualTo("simple"); + assertThat(InternalPropertyImpl.create(SymbolicName.unresolved(), "simple").getName()).isEqualTo("simple"); } @Test void multiplePropertyGetNameShouldWork() { - assertThat(Property.create(SymbolicName.unresolved(), "foo", "bar").getName()).isEqualTo("foo.bar"); + assertThat(InternalPropertyImpl.create(SymbolicName.unresolved(), "foo", "bar").getName()).isEqualTo("foo.bar"); } @Test void externalReferenceHasPriority() { - assertThat(Property.create(SymbolicName.unresolved(), "foo", "bar").referencedAs("something") + assertThat(InternalPropertyImpl.create(SymbolicName.unresolved(), "foo", "bar").referencedAs("something") .getName()).isEqualTo("something"); } }