Skip to content

Commit

Permalink
[3Hl7fTsQ] Dev fix misleading name column (#675)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gem-neo4j
gem-neo4j authored Nov 6, 2024
1 parent d8ffa0b commit f0c8172
Show file tree
Hide file tree
Showing 11 changed files with 432 additions and 120 deletions.
28 changes: 23 additions & 5 deletions core/src/main/java/apoc/help/Help.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.util.Set;
import java.util.stream.Stream;
import org.neo4j.graphdb.Transaction;
import org.neo4j.kernel.api.QueryLanguage;
import org.neo4j.kernel.api.procedure.QueryLanguageScope;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Description;
import org.neo4j.procedure.Name;
Expand Down Expand Up @@ -59,10 +61,25 @@ public Help() {

@NotThreadSafe
@Procedure("apoc.help")
@QueryLanguageScope(scope = {QueryLanguage.CYPHER_5})
@Description(
"Returns descriptions of the available APOC procedures and functions. If a keyword is provided, it will return only those procedures and functions that have the keyword in their name.")
public Stream<HelpResult> info(
public Stream<HelpResult> infoCypher5(
@Name(value = "proc", description = "A keyword to filter the results by.") String name) {
return help(name, true);
}

@NotThreadSafe
@Procedure("apoc.help")
@QueryLanguageScope(scope = {QueryLanguage.CYPHER_25})
@Description(
"Returns descriptions of the available APOC procedures and functions. If a keyword is provided, it will return only those procedures and functions that have the keyword in their name.")
public Stream<HelpResult> infoCypher25(
@Name(value = "proc", description = "A keyword to filter the results by.") String name) {
return help(name, false);
}

private Stream<HelpResult> help(String name, Boolean version5) {
boolean searchText = false;
if (name != null) {
name = name.trim();
Expand All @@ -71,15 +88,16 @@ public Stream<HelpResult> info(
searchText = true;
}
}
String CypherPreparser = version5 ? "CYPHER 5 " : "CYPHER 25 ";
String filter =
" WHERE name starts with 'apoc.' " + " AND ($name IS NULL OR toLower(name) CONTAINS toLower($name) "
+ " OR ($desc IS NOT NULL AND toLower(description) CONTAINS toLower($desc))) ";

String proceduresQuery = "SHOW PROCEDURES yield name, description, signature, isDeprecated " + filter
+ "RETURN 'procedure' as type, name, description, signature, isDeprecated ";
String proceduresQuery = CypherPreparser + "SHOW PROCEDURES yield name, description, signature, isDeprecated "
+ filter + "RETURN 'procedure' as type, name, description, signature, isDeprecated ";

String functionsQuery = "SHOW FUNCTIONS yield name, description, signature, isDeprecated " + filter
+ "RETURN 'function' as type, name, description, signature, isDeprecated ";
String functionsQuery = CypherPreparser + "SHOW FUNCTIONS yield name, description, signature, isDeprecated "
+ filter + "RETURN 'function' as type, name, description, signature, isDeprecated ";
Map<String, Object> params = map("name", name, "desc", searchText ? name : null);
Stream<Map<String, Object>> proceduresResults = tx.execute(proceduresQuery, params).stream();
Stream<Map<String, Object>> functionsResults = tx.execute(functionsQuery, params).stream();
Expand Down
88 changes: 72 additions & 16 deletions core/src/main/java/apoc/schema/Schemas.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@
import org.neo4j.internal.kernel.api.exceptions.schema.IndexNotFoundKernelException;
import org.neo4j.internal.schema.IndexDescriptor;
import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.QueryLanguage;
import org.neo4j.kernel.api.Statement;
import org.neo4j.kernel.api.procedure.QueryLanguageScope;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Description;
import org.neo4j.procedure.Mode;
Expand Down Expand Up @@ -107,6 +109,29 @@ public Stream<AssertSchemaResult> schemaAssert(

@NotThreadSafe
@Procedure(name = "apoc.schema.nodes", mode = Mode.SCHEMA)
@QueryLanguageScope(scope = {QueryLanguage.CYPHER_5})
@Description("Returns all indexes and constraints information for all `NODE` labels in the database.\n"
+ "It is possible to define a set of labels to include or exclude in the config parameters.")
public Stream<IndexConstraintNodeInfo> schemaNodesCypher5(
@Name(
value = "config",
defaultValue = "{}",
description =
"""
{
labels :: LIST<STRING>,
excludeLabels :: LIST<STRING>,
relationships :: LIST<STRING>,
excludeRelationships :: LIST<STRING>
}
""")
Map<String, Object> config) {
return indexesAndConstraintsForNode(config, false);
}

@NotThreadSafe
@Procedure(name = "apoc.schema.nodes", mode = Mode.SCHEMA)
@QueryLanguageScope(scope = {QueryLanguage.CYPHER_25})
@Description("Returns all indexes and constraints information for all `NODE` labels in the database.\n"
+ "It is possible to define a set of labels to include or exclude in the config parameters.")
public Stream<IndexConstraintNodeInfo> nodes(
Expand All @@ -123,11 +148,34 @@ public Stream<IndexConstraintNodeInfo> nodes(
}
""")
Map<String, Object> config) {
return indexesAndConstraintsForNode(config);
return indexesAndConstraintsForNode(config, true);
}

@NotThreadSafe
@Procedure(name = "apoc.schema.relationships", mode = Mode.SCHEMA)
@QueryLanguageScope(scope = {QueryLanguage.CYPHER_5})
@Description("Returns the indexes and constraints information for all the relationship types in the database.\n"
+ "It is possible to define a set of relationship types to include or exclude in the config parameters.")
public Stream<IndexConstraintRelationshipInfo> schemaRelationshipsCypher5(
@Name(
value = "config",
defaultValue = "{}",
description =
"""
{
labels :: LIST<STRING>,
excludeLabels :: LIST<STRING>,
relationships :: LIST<STRING>,
excludeRelationships :: LIST<STRING>
}
""")
Map<String, Object> config) {
return indexesAndConstraintsForRelationships(config, false);
}

@NotThreadSafe
@Procedure(name = "apoc.schema.relationships", mode = Mode.SCHEMA)
@QueryLanguageScope(scope = {QueryLanguage.CYPHER_25})
@Description("Returns the indexes and constraints information for all the relationship types in the database.\n"
+ "It is possible to define a set of relationship types to include or exclude in the config parameters.")
public Stream<IndexConstraintRelationshipInfo> relationships(
Expand All @@ -144,7 +192,7 @@ public Stream<IndexConstraintRelationshipInfo> relationships(
}
""")
Map<String, Object> config) {
return indexesAndConstraintsForRelationships(config);
return indexesAndConstraintsForRelationships(config, true);
}

@NotThreadSafe
Expand Down Expand Up @@ -429,7 +477,8 @@ private Boolean constraintsExistsForRelationship(String type, List<String> prope
*
* @return
*/
private Stream<IndexConstraintNodeInfo> indexesAndConstraintsForNode(Map<String, Object> config) {
private Stream<IndexConstraintNodeInfo> indexesAndConstraintsForNode(
Map<String, Object> config, Boolean useStoredName) {
Schema schema = tx.schema();

SchemaConfig schemaConfig = new SchemaConfig(config);
Expand Down Expand Up @@ -490,12 +539,14 @@ private Stream<IndexConstraintNodeInfo> indexesAndConstraintsForNode(Map<String,

Stream<IndexConstraintNodeInfo> constraintNodeInfoStream = StreamSupport.stream(
constraintsIterator.spliterator(), false)
.map(constraintDescriptor -> nodeInfoFromConstraintDefinition(constraintDescriptor, tokenRead))
.map(constraintDescriptor ->
nodeInfoFromConstraintDefinition(constraintDescriptor, tokenRead, useStoredName))
.sorted(Comparator.comparing(i -> i.label.toString()));

Stream<IndexConstraintNodeInfo> indexNodeInfoStream = StreamSupport.stream(
indexesIterator.spliterator(), false)
.map(indexDescriptor -> this.nodeInfoFromIndexDefinition(indexDescriptor, schemaRead, tokenRead))
.map(indexDescriptor ->
this.nodeInfoFromIndexDefinition(indexDescriptor, schemaRead, tokenRead, useStoredName))
.sorted(Comparator.comparing(i -> i.label.toString()));

return Stream.of(constraintNodeInfoStream, indexNodeInfoStream).flatMap(e -> e);
Expand All @@ -514,7 +565,8 @@ private List<IndexDescriptor> getIndexesFromSchema(
*
* @return
*/
private Stream<IndexConstraintRelationshipInfo> indexesAndConstraintsForRelationships(Map<String, Object> config) {
private Stream<IndexConstraintRelationshipInfo> indexesAndConstraintsForRelationships(
Map<String, Object> config, Boolean useStoredName) {
Schema schema = tx.schema();

SchemaConfig schemaConfig = new SchemaConfig(config);
Expand Down Expand Up @@ -570,11 +622,11 @@ private Stream<IndexConstraintRelationshipInfo> indexesAndConstraintsForRelation

Stream<IndexConstraintRelationshipInfo> constraintRelationshipInfoStream = StreamSupport.stream(
constraintsIterator.spliterator(), false)
.map(this::relationshipInfoFromConstraintDefinition);
.map(c -> relationshipInfoFromConstraintDefinition(c, useStoredName));

Stream<IndexConstraintRelationshipInfo> indexRelationshipInfoStream = StreamSupport.stream(
indexesIterator.spliterator(), false)
.map(index -> relationshipInfoFromIndexDescription(index, tokenRead, schemaRead));
.map(index -> relationshipInfoFromIndexDescription(index, tokenRead, schemaRead, useStoredName));

return Stream.of(constraintRelationshipInfoStream, indexRelationshipInfoStream)
.flatMap(e -> e);
Expand All @@ -589,12 +641,14 @@ private Stream<IndexConstraintRelationshipInfo> indexesAndConstraintsForRelation
* @return
*/
private IndexConstraintNodeInfo nodeInfoFromConstraintDefinition(
ConstraintDefinition constraintDefinition, TokenNameLookup tokens) {
ConstraintDefinition constraintDefinition, TokenNameLookup tokens, Boolean useStoredName) {
String labelName = constraintDefinition.getLabel().name();
List<String> properties = Iterables.asList(constraintDefinition.getPropertyKeys());
return new IndexConstraintNodeInfo(
// Pretty print for index name
String.format(":%s(%s)", labelName, StringUtils.join(properties, ",")),
useStoredName
? constraintDefinition.getName()
: String.format(":%s(%s)", labelName, StringUtils.join(properties, ",")),
labelName,
properties,
StringUtils.EMPTY,
Expand All @@ -617,7 +671,7 @@ private IndexConstraintNodeInfo nodeInfoFromConstraintDefinition(
* @return
*/
private IndexConstraintNodeInfo nodeInfoFromIndexDefinition(
IndexDescriptor indexDescriptor, SchemaRead schemaRead, TokenNameLookup tokens) {
IndexDescriptor indexDescriptor, SchemaRead schemaRead, TokenNameLookup tokens, Boolean useStoredName) {
int[] labelIds = indexDescriptor.schema().getEntityTokenIds();
int length = labelIds.length;
final Object labelName;
Expand All @@ -640,7 +694,7 @@ private IndexConstraintNodeInfo nodeInfoFromIndexDefinition(
final String userDescription = indexDescriptor.userDescription(tokens);
try {
return new IndexConstraintNodeInfo(
schemaInfoName,
useStoredName ? indexDescriptor.getName() : schemaInfoName,
labelName,
properties,
schemaRead.indexGetState(indexDescriptor).toString(),
Expand Down Expand Up @@ -668,7 +722,7 @@ private IndexConstraintNodeInfo nodeInfoFromIndexDefinition(
}

private IndexConstraintRelationshipInfo relationshipInfoFromIndexDescription(
IndexDescriptor indexDescriptor, TokenNameLookup tokens, SchemaRead schemaRead) {
IndexDescriptor indexDescriptor, TokenNameLookup tokens, SchemaRead schemaRead, Boolean useStoredName) {
int[] relIds = indexDescriptor.schema().getEntityTokenIds();
int length = relIds.length;
// to handle LOOKUP indexes
Expand All @@ -687,7 +741,7 @@ private IndexConstraintRelationshipInfo relationshipInfoFromIndexDescription(
.collect(Collectors.toList());

// Pretty print for index name
final String name = getSchemaInfoName(relName, properties);
final String name = useStoredName ? indexDescriptor.getName() : getSchemaInfoName(relName, properties);
final String schemaType = getIndexType(indexDescriptor);

String indexStatus;
Expand All @@ -707,9 +761,11 @@ private IndexConstraintRelationshipInfo relationshipInfoFromIndexDescription(
* @return
*/
private IndexConstraintRelationshipInfo relationshipInfoFromConstraintDefinition(
ConstraintDefinition constraintDefinition) {
ConstraintDefinition constraintDefinition, Boolean useStoredName) {
return new IndexConstraintRelationshipInfo(
String.format("CONSTRAINT %s", constraintDefinition.toString()),
useStoredName
? constraintDefinition.getName()
: String.format("CONSTRAINT %s", constraintDefinition.toString()),
constraintDefinition.getConstraintType().name(),
Iterables.asList(constraintDefinition.getPropertyKeys()),
"",
Expand Down
89 changes: 88 additions & 1 deletion core/src/test/java/apoc/schema/SchemasTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.neo4j.configuration.GraphDatabaseInternalSettings;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.graphdb.Label;
import org.neo4j.graphdb.QueryExecutionException;
Expand All @@ -69,7 +70,8 @@ public class SchemasTest {

@Rule
public DbmsRule db = new ImpermanentDbmsRule()
.withSetting(GraphDatabaseSettings.procedure_unrestricted, Collections.singletonList("apoc.*"));
.withSetting(GraphDatabaseSettings.procedure_unrestricted, Collections.singletonList("apoc.*"))
.withSetting(GraphDatabaseInternalSettings.enable_experimental_cypher_versions, true);

private static void accept(Result result) {
Map<String, Object> r = result.next();
Expand Down Expand Up @@ -862,6 +864,91 @@ public void testUniqueRelationshipConstraint() {
});
}

@Test
public void testCypher5Vs25RelNaming() {
db.executeTransactionally("CREATE CONSTRAINT FOR ()-[since:SINCE]-() REQUIRE since.year IS UNIQUE");
db.executeTransactionally("CREATE CONSTRAINT gemTest FOR ()-[for:FOR]-() REQUIRE for.year IS UNIQUE");
db.executeTransactionally("CREATE LOOKUP INDEX rel_type_lookup_index FOR ()-[r]-() ON EACH type(r)");
awaitIndexesOnline();

var cypher5Names = List.of(
"CONSTRAINT FOR ()-[since:SINCE]-() REQUIRE since.year IS UNIQUE",
"CONSTRAINT FOR ()-[for:FOR]-() REQUIRE for.year IS UNIQUE",
":FOR(year)",
":SINCE(year)",
":<any-types>()");

var cypher25Names = List.of("gemTest", "gemTest", "rel_type_lookup_index");

testResult(db, "CYPHER 5 CALL apoc.schema.relationships({})", result -> {
List<String> producedNames = new ArrayList<>();
while (result.hasNext()) {
Map<String, Object> r = result.next();
producedNames.add((String) r.get("name"));
}

assertTrue(producedNames.containsAll(cypher5Names));
assertEquals(5, producedNames.size());
});

testResult(db, "CYPHER 25 CALL apoc.schema.relationships({})", result -> {
List<String> producedNames = new ArrayList<>();
while (result.hasNext()) {
Map<String, Object> r = result.next();
producedNames.add((String) r.get("name"));
}

assertTrue(producedNames.containsAll(cypher25Names));
assertEquals(
2,
producedNames.stream()
.filter(c -> c.startsWith("constraint_"))
.toList()
.size());
assertEquals(5, producedNames.size());
});
}

@Test
public void testCypher5Vs25NodeNaming() {
db.executeTransactionally("CREATE CONSTRAINT FOR (book:Book) REQUIRE book.isbn IS UNIQUE");
db.executeTransactionally("CREATE CONSTRAINT gemTest FOR (library:Library) REQUIRE library.name IS UNIQUE");
db.executeTransactionally("CREATE LOOKUP INDEX node_label_lookup_index FOR (n) ON EACH labels(n)");
awaitIndexesOnline();

var cypher5Names = List.of(":Book(isbn)", ":Library(name)", ":Book(isbn)", ":Library(name)", ":<any-labels>()");

var cypher25Names = List.of("gemTest", "gemTest", "node_label_lookup_index");

testResult(db, "CYPHER 5 CALL apoc.schema.nodes({})", result -> {
List<String> producedNames = new ArrayList<>();
while (result.hasNext()) {
Map<String, Object> r = result.next();
producedNames.add((String) r.get("name"));
}

assertTrue(producedNames.containsAll(cypher5Names));
assertEquals(5, producedNames.size());
});

testResult(db, "CYPHER 25 CALL apoc.schema.nodes({})", result -> {
List<String> producedNames = new ArrayList<>();
while (result.hasNext()) {
Map<String, Object> r = result.next();
producedNames.add((String) r.get("name"));
}

assertTrue(producedNames.containsAll(cypher25Names));
assertEquals(
2,
producedNames.stream()
.filter(c -> c.startsWith("constraint_"))
.toList()
.size());
assertEquals(5, producedNames.size());
});
}

@Test
public void testMultipleUniqueRelationshipConstraints() {
db.executeTransactionally("CREATE CONSTRAINT FOR ()-[since:SINCE]-() REQUIRE since.year IS UNIQUE");
Expand Down
Loading

0 comments on commit f0c8172

Please sign in to comment.