Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: fix TinkerPop unit test #2055

Merged
merged 14 commits into from
Dec 26, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,20 @@ jobs:
distribution: 'zulu'

- name: Cache Maven packages
uses: actions/cache@v2
uses: actions/cache@v3
with:
path: ~/.m2
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
restore-keys: ${{ runner.os }}-m2

- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
fetch-depth: 2

- name: Package
run: |
mvn clean package -DskipTests
mvn clean package -DskipTests -ntp

- name: Rename file
if: contains(env.TAG_NAME, "-")
Expand Down
14 changes: 1 addition & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
distribution: 'zulu'

- name: Cache Maven packages
uses: actions/cache@v2
uses: actions/cache@v3
with:
path: ~/.m2
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
Expand Down Expand Up @@ -68,18 +68,6 @@ jobs:
java-version: ${{ matrix.JAVA_VERSION }}
distribution: 'zulu'

# - name: Init MySQL Env
# if: ${{ env.BACKEND == 'mysql' }}
# uses: mirromutth/mysql-action@v1.1
# with:
# host port: 3306 # Optional, default value is 3306. The port of host
# container port: 3306 # Optional, default value is 3306. The port of container
# character set server: 'utf8mb4' # Optional, default value is 'utf8mb4'. The '--character-set-server' option for mysqld
# collation server: 'utf8mb4_general_ci' # Optional, default value is 'utf8mb4_general_ci'. The '--collation-server' option for mysqld
# mysql version: '5.7' # Optional, default value is "latest". The version of the MySQL
# mysql database: 'test' # Optional, default value is "test". The specified database which will be create
# mysql root password: "******" # Required if "mysql user" is empty, default is empty. The root superuser password

- name: Run unit test
run: |
$TRAVIS_DIR/run-unit-test.sh $BACKEND
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@
import org.apache.hugegraph.util.E;
import org.apache.hugegraph.util.InsertionOrderUtil;
import org.apache.hugegraph.util.NumericUtil;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;

public final class ConditionQueryFlatten {

private static final Set<HugeKeys> SPECIAL_KEYS = ImmutableSet.of(
HugeKeys.LABEL
HugeKeys.LABEL
);

public static List<ConditionQuery> flatten(ConditionQuery query) {
Expand Down Expand Up @@ -123,8 +124,8 @@ private static Condition flattenIn(Condition condition, boolean supportIn) {
return new Condition.Or(flattenIn(or.left(), supportIn),
flattenIn(or.right(), supportIn));
default:
throw new AssertionError(String.format(
"Wrong condition type: '%s'", condition.type()));
throw new AssertionError(String.format("Wrong condition type: '%s'",
condition.type()));
}
}

Expand All @@ -144,8 +145,7 @@ private static Condition convIn2Or(Relation relation, boolean supportIn) {
(key == HugeKeys.OWNER_VERTEX || key == HugeKeys.ID)) {
// TODO: Should not rely on HugeKeys here, improve key judgment
// Just mark flatten
return new Condition.FlattenSyspropRelation(
(SyspropRelation) relation);
return new Condition.FlattenSyspropRelation((SyspropRelation) relation);
}

// Do IN flatten, return null if values.size() == 0
Expand Down Expand Up @@ -214,8 +214,8 @@ private static Set<Relations> flattenAndOr(Condition condition) {
flattenAndOr(or.right()));
break;
default:
throw new AssertionError(String.format(
"Wrong condition type: '%s'", condition.type()));
throw new AssertionError(String.format("Wrong condition type: '%s'",
condition.type()));
}
return result;
}
Expand Down Expand Up @@ -308,8 +308,9 @@ private static Relations optimizeRelations(Relations relations) {

/**
* Reduce and merge relations linked with 'AND' for same key
* @param relations linked with 'AND' having same key, may contains 'in',
* 'not in', '>', '<', '>=', '<=', '==', '!='
*
* @param relations linked with 'AND' having same key, may contain 'in', 'not in',
* '>', '<', '>=', '<=', '==', '!='
* @return merged relations
*/
private static Relations mergeRelations(Relations relations) {
Expand Down Expand Up @@ -429,8 +430,8 @@ private static boolean validRange(Relation low, Relation high) {
return true;
}
return compare(low, high) < 0 || compare(low, high) == 0 &&
low.relation() == Condition.RelationType.GTE &&
high.relation() == Condition.RelationType.LTE;
low.relation() == Condition.RelationType.GTE &&
high.relation() == Condition.RelationType.LTE;
}

private static boolean validEq(Relation eq, Relation low, Relation high) {
Expand Down Expand Up @@ -479,8 +480,7 @@ private static Relation lowRelation(Relation first, Relation second) {
return selectRelation(first, second, false);
}

private static Relation selectRelation(Relation first, Relation second,
boolean high) {
private static Relation selectRelation(Relation first, Relation second, boolean high) {
if (first == null) {
return second;
}
Expand Down Expand Up @@ -510,8 +510,8 @@ private static int compare(Relation first, Relation second) {
} else if (firstValue instanceof Date && secondValue instanceof Date) {
return ((Date) firstValue).compareTo((Date) secondValue);
} else {
throw new IllegalArgumentException(String.format(
"Can't compare between %s and %s", first, second));
throw new IllegalArgumentException(String.format("Can't compare between %s and %s",
first, second));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,13 @@ private boolean hasIds() {
}

private Query makeQuery(HugeGraph graph, HugeType type) {
Query query = null;
Query query;
if (this.hasContainers.isEmpty()) {
// Query all
query = new Query(type);
} else {
ConditionQuery q = new ConditionQuery(type);
query = TraversalUtil.fillConditionQuery(q, this.hasContainers,
graph);
query = TraversalUtil.fillConditionQuery(q, this.hasContainers, graph);
}

query = this.injectQueryInfo(query);
Expand Down Expand Up @@ -202,6 +201,7 @@ public Iterator<?> lastTimeResults() {
return this.lastTimeResults;
}

@Override
public boolean equals(Object obj) {
if (!(obj instanceof HugeGraphStep)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,21 +107,17 @@ public static HugeGraph getGraph(Step<?, ?> step) {
}

public static HugeGraph tryGetGraph(Step<?, ?> step) {
// TODO: remove these EmptyGraph judgments when upgrading tinkerpop to a fixed version tinkerpop#1699
// TODO: remove these EmptyGraph judgments when upgrade tinkerpop (refer-tinkerpop#1699)
Optional<Graph> graph = step.getTraversal()
.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
});
.filter(g -> !(g instanceof EmptyGraph));
if (!graph.isPresent()) {
TraversalParent parent = step.getTraversal().getParent();
if (parent instanceof Traversal) {
Optional<Graph> parentGraph = ((Traversal<?, ?>) parent)
.asAdmin()
.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
});
Optional<Graph> parentGraph;
parentGraph = ((Traversal<?, ?>) parent).asAdmin()
.getGraph()
.filter(g -> !(g instanceof EmptyGraph));
if (parentGraph.isPresent()) {
step.getTraversal().setGraph(parentGraph.get());
return (HugeGraph) parentGraph.get();
Expand All @@ -140,28 +136,20 @@ public static void trySetGraph(Step<?, ?> step, HugeGraph graph) {
return;
}

// TODO: remove these EmptyGraph judgments when upgrading tinkerpop to a fixed version tinkerpop#1699
// TODO: remove these EmptyGraph judgments when upgrade tinkerpop (refer-tinkerpop#1699)
Optional<Graph> stepGraph = step.getTraversal()
.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
});
.filter(g -> !(g instanceof EmptyGraph));

if (step instanceof TraversalParent) {
for (final Traversal.Admin<?, ?> local : ((TraversalParent) step).getLocalChildren()) {
if (local.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
if (local.getGraph().filter(g -> !(g instanceof EmptyGraph)).isPresent()) {
continue;
}
local.setGraph(graph);
}
for (final Traversal.Admin<?, ?> global : ((TraversalParent) step).getGlobalChildren()) {
if (global.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
if (global.getGraph().filter(g -> !(g instanceof EmptyGraph)).isPresent()) {
continue;
}
global.setGraph(graph);
Expand Down Expand Up @@ -217,7 +205,6 @@ public static void extractOrder(Step<?, ?> newStep,
step = step.getNextStep();
if (step instanceof OrderGlobalStep) {
QueryHolder holder = (QueryHolder) newStep;
@SuppressWarnings("resource")
OrderGlobalStep<?, ?> orderStep = (OrderGlobalStep<?, ?>) step;
orderStep.getComparators().forEach(comp -> {
ElementValueComparator<?> comparator =
Expand Down Expand Up @@ -288,7 +275,6 @@ public static void extractAggregateFunc(Step<?, ?> newStep,
do {
step = step.getNextStep();
if (step instanceof PropertiesStep) {
@SuppressWarnings("resource")
PropertiesStep<?> propStep = (PropertiesStep<?>) step;
if (propStep.getReturnType() == PropertyType.VALUE &&
propStep.getPropertyKeys().length == 1) {
Expand Down Expand Up @@ -653,20 +639,15 @@ public static void convAllHasSteps(Traversal.Admin<?, ?> traversal) {
* `g.V().hasLabel('person').union(__.has('name', 'tom'))`
* Here `__.has()` will create a new traversal, but the graph is null
*/
if (!traversal.getGraph()
.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
if (!traversal.getGraph().filter(g -> !(g instanceof EmptyGraph)).isPresent()) {
if (traversal.getParent() == null || !(traversal.getParent() instanceof Traversal)) {
return;
}

Optional<Graph> parentGraph = ((Traversal<?, ?>) traversal.getParent())
.asAdmin()
.getGraph();
if (parentGraph.filter(g -> {
return !(g instanceof EmptyGraph);
}).isPresent()) {
if (parentGraph.filter(g -> !(g instanceof EmptyGraph)).isPresent()) {
traversal.setGraph(parentGraph.get());
}
}
Expand All @@ -686,7 +667,7 @@ public static void convHasStep(HugeGraph graph, HasStep<?> step) {

private static void convPredicateValue(HugeGraph graph,
HasContainer has) {
// No need to convert if key is sysprop
// No need to convert if key is sys-prop
if (isSysProp(has.getKey())) {
return;
}
Expand Down Expand Up @@ -802,8 +783,7 @@ private static <V> V validPropertyValue(V value, PropertyKey pkey) {

public static void retrieveSysprop(List<HasContainer> hasContainers,
Function<HasContainer, Boolean> func) {
for (Iterator<HasContainer> iter = hasContainers.iterator();
iter.hasNext();) {
for (Iterator<HasContainer> iter = hasContainers.iterator(); iter.hasNext();) {
HasContainer container = iter.next();
if (container.getKey().startsWith("~") && func.apply(container)) {
iter.remove();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@

public class MultiGraphsTest extends BaseCoreTest {

private static final String NAME48 =
"g12345678901234567890123456789012345678901234567";
private static final String NAME48 = "g12345678901234567890123456789012345678901234567";

@Test
public void testWriteAndReadVersion() {
Expand Down Expand Up @@ -233,16 +232,12 @@ public void testCopySchemaWithMultiGraphsWithConflict() {

@Test
public void testCreateGraphsWithInvalidNames() {
List<String> invalidNames = ImmutableList.of(
"", " ", " g", "g 1", " .", ". .",
"@", "$", "%", "^", "&", "*", "(", ")",
"_", "+", "`", "-", "=", "{", "}", "|",
"[", "]", "\"", "<", "?", ";", "'", "~",
",", ".", "/", "\\",
"~g", "g~", "g'",
"_1", "_a",
"1a", "123",
NAME48 + "8");
List<String> invalidNames = ImmutableList.of("", " ", " g", "g 1", " .", ". .",
"@", "$", "%", "^", "&", "*", "(", ")",
"_", "+", "`", "-", "=", "{", "}", "|",
"[", "]", "\"", "<", "?", ";", "'", "~",
",", ".", "/", "\\", "~g", "g~", "g'",
"_1", "_a", "1a", "123", NAME48 + "8");
for (String name : invalidNames) {
Assert.assertThrows(RuntimeException.class, () -> openGraphs(name));
}
Expand All @@ -267,8 +262,7 @@ public void testCreateGraphsWithSameName() {
() -> g2.vertexLabel("node"));
Assert.assertThrows(IllegalArgumentException.class,
() -> g3.vertexLabel("node"));
g1.schema().vertexLabel("node").useCustomizeNumberId()
.ifNotExist().create();
g1.schema().vertexLabel("node").useCustomizeNumberId().ifNotExist().create();
g2.vertexLabel("node");
g3.vertexLabel("node");

Expand All @@ -294,13 +288,11 @@ public void testCreateGraphsWithSameName() {
}

@Test
public void testCreateGraphWithSameNameDifferentBackends()
throws Exception {
public void testCreateGraphWithSameNameDifferentBackends() throws Exception {
HugeGraph g1 = openGraphWithBackend("graph", "memory", "text");
g1.initBackend();
Assert.assertThrows(RuntimeException.class,
() -> openGraphWithBackend("graph", "rocksdb",
"binary"));
() -> openGraphWithBackend("graph", "rocksdb", "binary"));
g1.clearBackend();
g1.close();
}
Expand All @@ -323,17 +315,15 @@ public void testCreateGraphsWithDifferentNameDifferentBackends() {

@Test
public void testCreateGraphsWithMultiDisksForRocksDB() {
HugeGraph g1 = openGraphWithBackend(
"g1", "rocksdb", "binary",
"rocksdb.data_disks",
"[g/secondary_index:rocksdb-index1," +
" g/range_int_index:rocksdb-index1," +
" g/range_long_index:rocksdb-index2]");
HugeGraph g1 = openGraphWithBackend("g1", "rocksdb", "binary", "rocksdb.data_disks",
"[g/secondary_index:rocksdb-index1," +
" g/range_int_index:rocksdb-index1," +
" g/range_long_index:rocksdb-index2]");
g1.initBackend();
g1.clearBackend();

final HugeGraph[] g2 = new HugeGraph[1];
Assert.assertThrows(ConnectionException.class, () -> {
Assert.assertThrows(RuntimeException.class, () -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep the ConnectionException and update the throw-exception location?

g2[0] = openGraphWithBackend("g2", "rocksdb", "binary",
"rocksdb.data_disks",
"[g/range_int_index:rocksdb-index1]");
Expand All @@ -348,7 +338,7 @@ public void testCreateGraphsWithMultiDisksForRocksDB() {
});

final HugeGraph[] g3 = new HugeGraph[1];
Assert.assertThrows(ConnectionException.class, () -> {
Assert.assertThrows(RuntimeException.class, () -> {
g3[0] = openGraphWithBackend("g3", "rocksdb", "binary",
"rocksdb.data_disks",
"[g/secondary_index:/]");
Expand Down Expand Up @@ -388,7 +378,7 @@ private static HugeGraph openGraphWithBackend(String graph, String backend,
Configuration config = buildConfig(graph);
config.setProperty(CoreOptions.BACKEND.name(), backend);
config.setProperty(CoreOptions.SERIALIZER.name(), serializer);
for (int i = 0; i < configs.length;) {
for (int i = 0; i < configs.length; ) {
config.setProperty(configs[i++], configs[i++]);
}
return ((HugeGraph) GraphFactory.open(config));
Expand All @@ -397,7 +387,7 @@ private static HugeGraph openGraphWithBackend(String graph, String backend,
private static Configuration buildConfig(String graphName) {
PropertiesConfiguration conf = Utils.getConf();
Configuration config = new BaseConfiguration();
for (Iterator<String> keys = conf.getKeys(); keys.hasNext();) {
for (Iterator<String> keys = conf.getKeys(); keys.hasNext(); ) {
String key = keys.next();
config.setProperty(key, conf.getProperty(key));
}
Expand Down