gchqGh-3210: Fix GafferPopVertex.properties() output (gchq#3211)

* gchqGh-3210 Fix GafferPopVertex.properties() output Set a proper ID for GafferPopVertexProperties so that equals and hashCode work as expected simplify GafferPopVertex.properties() method add unit test to catch error
GCHQDeveloper314 · May 20, 2024 · f6928b3 · f6928b3
1 parent 759531c
commit f6928b3
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 25 deletions.
diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopVertex.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopVertex.java
@@ -23,7 +23,6 @@
 import org.apache.tinkerpop.gremlin.structure.VertexProperty;
 import org.apache.tinkerpop.gremlin.structure.util.ElementHelper;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
-import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -38,7 +37,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 /**
  * A <code>GafferPopEdge</code> is an {@link GafferPopElement} and {@link Vertex}.
@@ -82,26 +80,13 @@ public <V> VertexProperty<V> property(final VertexProperty.Cardinality cardinali
 
     @Override
     public <V> Iterator<VertexProperty<V>> properties(final String... propertyKeys) {
-        if (properties == null) {
-            return Collections.emptyIterator();
-        }
-        if (propertyKeys.length == 1) {
-            final List<VertexProperty> props = properties.getOrDefault(propertyKeys[0], Collections.emptyList());
-            if (props.size() == 1) {
-                return IteratorUtils.of(props.get(0));
-            } else if (props.isEmpty()) {
-                return Collections.emptyIterator();
-            } else {
-                return (Iterator) new ArrayList<>(props).iterator();
-            }
-        } else {
-            return (Iterator) properties.entrySet()
-                    .stream()
-                    .filter(entry -> ElementHelper.keyExists(entry.getKey(), propertyKeys))
-                    .flatMap(entry -> entry.getValue().stream())
-                    .collect(Collectors.toList())
-                    .iterator();
-        }
+        return properties == null ?
+                Collections.emptyIterator() :
+                (Iterator) properties.entrySet()
+                        .stream()
+                        .filter(entry -> ElementHelper.keyExists(entry.getKey(), propertyKeys))
+                        .flatMap(entry -> entry.getValue().stream())
+                        .iterator();
     }
 
     /**

diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopVertexProperty.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopVertexProperty.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2023 Crown Copyright
+ * Copyright 2016-2024 Crown Copyright
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ public class GafferPopVertexProperty<V> extends GafferPopElement implements Vert
     private final V value;
 
     public GafferPopVertexProperty(final GafferPopVertex vertex, final String key, final V value, final Object... propertyKeyValues) {
-        super(vertex.label(), vertex.id(), vertex.graph());
+        super(key, String.format("[%s,%s,%s]", vertex.id(), key, value), vertex.graph());
         this.vertex = vertex;
         this.key = key;
         this.value = value;

diff --git a/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopVertexTest.java b/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopVertexTest.java
@@ -165,13 +165,16 @@ void shouldOnlyCreateValidGafferPopVertexPropertyObjects() {
         given(features.vertex()).willReturn(vertexFeatures);
         given(vertexFeatures.properties()).willReturn(vertexPropertyFeatures);
         final String propValue = "propValue";
+        final int intPropValue = 1;
         final String nestedKey = "nestedKey";
         final String nestedValue = "nestedValue";
         // Make new vertex with the mocked bits
         final GafferPopVertex vertex = new GafferPopVertex(TestGroups.ENTITY, GafferPopGraph.ID_LABEL, graph);
         // Make some values to compare against
         final GafferPopVertexProperty<Object> equalProp = new GafferPopVertexProperty<>(vertex, TestPropertyNames.STRING, propValue);
         final String notAProp = "notAProp";
+        final GafferPopVertexProperty<Object> notEqualProp = new GafferPopVertexProperty<>(vertex, TestPropertyNames.INT, intPropValue);
+
 
         // When
         // Set and get the property
@@ -185,7 +188,9 @@ void shouldOnlyCreateValidGafferPopVertexPropertyObjects() {
                 .isEqualTo(equalProp)
                 .hasSameHashCodeAs(equalProp)
                 .isNotEqualTo(notAProp)
-                .doesNotHaveSameHashCodeAs(notAProp);
+                .doesNotHaveSameHashCodeAs(notAProp)
+                .isNotEqualTo(notEqualProp)
+                .doesNotHaveSameHashCodeAs(notEqualProp);
         assertThat(prop.element()).isEqualTo(vertex);
         assertThat(prop.isPresent()).isTrue();
         assertThat(prop.keys()).isEmpty();