fix: support null value for gremlin test (#2061)

* fix(core): adapt HugeVertex.property() with null values Co-authored-by: imbajin <[email protected]>
apache · Jan 16, 2023 · 91a616b · 91a616b
1 parent f49b2b8
commit 91a616b
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 71 deletions.
diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
@@ -20,8 +20,8 @@ jobs:
         stale-pr-message: 'Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label'
         stale-issue-label: 'inactive'
         stale-pr-label: 'inactive'
-        exempt-issue-labels: 'feature,bug,enhancement,improvement,wontfix,todo,guide,doc,help wanted'
-        exempt-pr-labels: 'feature,bug,enhancement,improvement,wontfix,todo,guide,doc,help wanted'
+        exempt-issue-labels: 'feature,bug,enhancement,improvement,todo,guide,doc,help wanted,security'
+        exempt-pr-labels: 'feature,bug,enhancement,improvement,todo,guide,doc,help wanted,security'
         exempt-all-milestones: true
 
         days-before-issue-stale: 15

diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java b/hugegraph-core/src/main/java/org/apache/hugegraph/schema/PropertyKey.java
@@ -228,7 +228,7 @@ private <V> boolean checkDataType(V value) {
     }
 
     /**
-     * Check type of all the values(may be some of list properties) valid
+     * Check type of all the values(maybe some list properties) valid
      * @param values the property values to be checked data type
      * @param <V> the property value class
      * @return true if all the values are or can convert to the data type,

diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java b/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeEdge.java
@@ -45,12 +45,14 @@
 import org.apache.hugegraph.backend.serializer.BytesBuffer;
 import org.apache.hugegraph.perf.PerfUtil.Watched;
 import org.apache.hugegraph.util.E;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyProperty;
+
 import com.google.common.collect.ImmutableList;
 
 public class HugeEdge extends HugeElement implements Edge, Cloneable {
 
     private Id id;
-    private EdgeLabel label;
+    private final EdgeLabel label;
     private String name;
 
     private HugeVertex sourceVertex;
@@ -200,6 +202,11 @@ public <V> Property<V> property(String key, V value) {
         E.checkArgument(this.label.properties().contains(propertyKey.id()),
                         "Invalid property '%s' for edge label '%s'",
                         key, this.label());
+        if (value == null) {
+            this.removeProperty(propertyKey.id());
+            return EmptyProperty.instance();
+        }
+
         // Sort-Keys can only be set once
         if (this.schemaLabel().sortKeys().contains(propertyKey.id())) {
             E.checkArgument(!this.hasProperty(propertyKey.id()),
@@ -279,7 +286,7 @@ public <V> Iterator<Property<V>> properties(String... keys) {
 
         if (keys.length == 0) {
             for (HugeProperty<?> prop : this.getProperties()) {
-                assert prop instanceof Property;
+                assert prop != null;
                 props.add((Property<V>) prop);
             }
         } else {
@@ -295,7 +302,6 @@ public <V> Iterator<Property<V>> properties(String... keys) {
                     // Not found
                     continue;
                 }
-                assert prop instanceof Property;
                 props.add((Property<V>) prop);
             }
         }
@@ -320,8 +326,7 @@ public Object sysprop(HugeKeys key) {
             case PROPERTIES:
                 return this.getPropertiesMap();
             default:
-                E.checkArgument(false,
-                                "Invalid system property '%s' of Edge", key);
+                E.checkArgument(false, "Invalid system property '%s' of Edge", key);
                 return null;
         }
     }
@@ -362,6 +367,7 @@ public void vertices(HugeVertex owner, HugeVertex other) {
         if (ownerLabel.equals(this.label.sourceLabel())) {
             this.vertices(true, owner, other);
         } else {
+            // TODO: why compare the label but ignore the result?
             ownerLabel.equals(this.label.targetLabel());
             this.vertices(false, owner, other);
         }
@@ -515,8 +521,7 @@ public static HugeEdge constructEdge(HugeVertex ownerVertex,
             ownerVertex.correctVertexLabel(tgtLabel);
             otherVertexLabel = srcLabel;
         }
-        HugeVertex otherVertex = new HugeVertex(graph, otherVertexId,
-                                                otherVertexLabel);
+        HugeVertex otherVertex = new HugeVertex(graph, otherVertexId, otherVertexLabel);
 
         ownerVertex.propNotLoaded();
         otherVertex.propNotLoaded();

diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeElement.java b/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeElement.java
@@ -31,14 +31,19 @@
 import org.apache.hugegraph.backend.id.EdgeId;
 import org.apache.hugegraph.backend.id.Id;
 import org.apache.hugegraph.backend.id.IdGenerator;
+import org.apache.hugegraph.backend.serializer.BytesBuffer;
 import org.apache.hugegraph.backend.tx.GraphTransaction;
+import org.apache.hugegraph.perf.PerfUtil.Watched;
 import org.apache.hugegraph.schema.PropertyKey;
 import org.apache.hugegraph.schema.SchemaLabel;
 import org.apache.hugegraph.schema.VertexLabel;
 import org.apache.hugegraph.type.HugeType;
 import org.apache.hugegraph.type.Idfiable;
 import org.apache.hugegraph.type.define.Cardinality;
 import org.apache.hugegraph.type.define.HugeKeys;
+import org.apache.hugegraph.util.CollectionUtil;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.InsertionOrderUtil;
 import org.apache.hugegraph.util.collection.CollectionFactory;
 import org.apache.tinkerpop.gremlin.structure.Element;
 import org.apache.tinkerpop.gremlin.structure.Property;
@@ -47,12 +52,6 @@
 import org.eclipse.collections.api.iterator.IntIterator;
 import org.eclipse.collections.api.map.primitive.MutableIntObjectMap;
 
-import org.apache.hugegraph.backend.serializer.BytesBuffer;
-import org.apache.hugegraph.perf.PerfUtil.Watched;
-import org.apache.hugegraph.util.CollectionUtil;
-import org.apache.hugegraph.util.E;
-import org.apache.hugegraph.util.InsertionOrderUtil;
-
 public abstract class HugeElement implements Element, GraphType, Idfiable {
 
     private static final MutableIntObjectMap<HugeProperty<?>> EMPTY_MAP =
@@ -61,8 +60,8 @@ public abstract class HugeElement implements Element, GraphType, Idfiable {
 
     private final HugeGraph graph;
     private MutableIntObjectMap<HugeProperty<?>> properties;
-
-    private long expiredTime; // TODO: move into properties to keep small object
+    // TODO: move into properties to keep small object
+    private long expiredTime;
 
     private boolean removed;
     private boolean fresh;
@@ -425,14 +424,14 @@ public static final ElementKeys classifyKeys(Object... keyValues) {
             Object val = keyValues[i + 1];
 
             if (!(key instanceof String) && !(key instanceof T)) {
-                throw Element.Exceptions
-                      .providedKeyValuesMustHaveALegalKeyOnEvenIndices();
+                throw Element.Exceptions.providedKeyValuesMustHaveALegalKeyOnEvenIndices();
             }
             if (val == null) {
-                if (key.equals(T.label)) {
+                if (T.label.equals(key)) {
                     throw Element.Exceptions.labelCanNotBeNull();
                 }
-                throw Property.Exceptions.propertyDoesNotExist();
+                // Ignore null value for tinkerpop test compatibility
+                continue;
             }
 
             if (key.equals(T.id)) {

diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeProperty.java b/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeProperty.java
@@ -49,8 +49,7 @@ public PropertyKey propertyKey() {
     }
 
     public Object id() {
-        return SplicingIdGenerator.concat(this.owner.id().asString(),
-                                          this.key());
+        return SplicingIdGenerator.concat(this.owner.id().asString(), this.key());
     }
 
     @Override

diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeVertex.java b/hugegraph-core/src/main/java/org/apache/hugegraph/structure/HugeVertex.java
@@ -20,6 +20,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -59,6 +60,9 @@
 import org.apache.hugegraph.type.define.HugeKeys;
 import org.apache.hugegraph.type.define.IdStrategy;
 import org.apache.hugegraph.util.E;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyProperty;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyVertexProperty;
+
 import com.google.common.collect.ImmutableList;
 
 public class HugeVertex extends HugeElement implements Vertex, Cloneable {
@@ -162,8 +166,7 @@ public void assignId(Id id, boolean force) {
                 }
                 break;
             default:
-                throw new AssertionError(String.format(
-                          "Unknown id strategy '%s'", strategy));
+                throw new AssertionError(String.format("Unknown id strategy '%s'", strategy));
         }
         this.checkIdLength();
     }
@@ -250,7 +253,7 @@ public void addEdge(HugeEdge edge) {
 
     /**
      * Add one edge between this vertex and other vertex
-     *
+     * <p>
      * *** this method is not thread safe, must clone this vertex first before
      * multi thread access e.g. `vertex.copy().resetTx();` ***
      */
@@ -295,7 +298,7 @@ public HugeEdge constructEdge(String label, HugeVertex vertex,
                         label, this.label(), vertex.label());
         // Check sortKeys
         List<Id> keys = this.graph().mapPkName2Id(elemKeys.keys());
-        E.checkArgument(keys.containsAll(edgeLabel.sortKeys()),
+        E.checkArgument(new HashSet<>(keys).containsAll(edgeLabel.sortKeys()),
                         "The sort key(s) must be set for the edge " +
                         "with label: '%s'", edgeLabel.name());
 
@@ -304,7 +307,7 @@ public HugeEdge constructEdge(String label, HugeVertex vertex,
         Collection<Id> nonNullKeys = CollectionUtils.subtract(
                                      edgeLabel.properties(),
                                      edgeLabel.nullableKeys());
-        if (!keys.containsAll(nonNullKeys)) {
+        if (!new HashSet<>(keys).containsAll(nonNullKeys)) {
             @SuppressWarnings("unchecked")
             Collection<Id> missed = CollectionUtils.subtract(nonNullKeys, keys);
             E.checkArgument(false, "All non-null property keys: %s " +
@@ -419,9 +422,8 @@ public void remove() {
 
     @Watched(prefix = "vertex")
     @Override
-    public <V> VertexProperty<V> property(
-               VertexProperty.Cardinality cardinality,
-               String key, V value, Object... objects) {
+    public <V> VertexProperty<V> property(VertexProperty.Cardinality cardinality,
+                                          String key, V value, Object... objects) {
         if (objects.length != 0 && objects[0].equals(T.id)) {
             throw VertexProperty.Exceptions.userSuppliedIdsNotSupported();
         }
@@ -437,7 +439,7 @@ public <V> VertexProperty<V> property(
          *              .property(list, "key2", val2)
          *
          * The cardinality single may be user supplied single, it may also be
-         * that user doesn't supplied cardinality, when it is latter situation,
+         * that user doesn't supply cardinality, when it is latter situation,
          * we shouldn't check it. Because of this reason, we are forced to
          * give up the check of user supplied cardinality single.
          * The cardinality not single must be user supplied, so should check it
@@ -460,10 +462,14 @@ public <V> VertexProperty<V> property(
             E.checkArgument(!this.hasProperty(propertyKey.id()),
                             "Can't update primary key: '%s'", key);
         }
+        if (value == null) {
+            this.removeProperty(propertyKey.id());
+            return EmptyVertexProperty.instance();
+        }
 
         @SuppressWarnings("unchecked")
-        VertexProperty<V> prop = (VertexProperty<V>) this.addProperty(
-                                 propertyKey, value, !this.fresh());
+        VertexProperty<V> prop = (VertexProperty<V>) this.addProperty(propertyKey,
+                                                                      value, !this.fresh());
         return prop;
     }
 

diff --git a/hugegraph-test/src/main/java/org/apache/hugegraph/tinkerpop/ProcessStandardTest.java b/hugegraph-test/src/main/java/org/apache/hugegraph/tinkerpop/ProcessStandardTest.java
@@ -21,7 +21,6 @@
 import org.junit.runner.RunWith;
 
 @RunWith(ProcessBasicSuite.class)
-@GraphProviderClass(provider = ProcessTestGraphProvider.class,
-                    graph = TestGraph.class)
+@GraphProviderClass(provider = ProcessTestGraphProvider.class, graph = TestGraph.class)
 public class ProcessStandardTest {
 }