Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

Commit

Permalink
fix: turn on werror for neo4j-dao (linkedin#60)
Browse files Browse the repository at this point in the history 
I also got rid of the tuple library while I was here, easiler than trying to get the generics right :)
  • Loading branch information
John Plaisted committed Jan 14, 2021
1 parent d0d9095 commit c5faa27
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 80 deletions.
26 changes: 5 additions & 21 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ project.ext.externalDependency = [
'h2': 'com.h2database:h2:1.4.196',
'jacksonCore': 'com.fasterxml.jackson.core:jackson-core:2.9.7',
'jacksonDataBind': 'com.fasterxml.jackson.core:jackson-databind:2.9.7',
'javatuples': 'org.javatuples:javatuples:1.2',
'jsonSimple': 'com.googlecode.json-simple:json-simple:1.1.1',
'junitJupiterApi': "org.junit.jupiter:junit-jupiter-api:$junitJupiterVersion",
'junitJupiterParams': "org.junit.jupiter:junit-jupiter-params:$junitJupiterVersion",
Expand All @@ -70,32 +69,17 @@ project.ext.externalDependency = [
apply plugin: 'com.diffplug.spotless'
apply from: "./gradle/release.gradle"

// TODO expand this to all projects and then delete this allow list. This list is letting us fix errors over time rather
// than in one big change.
def wErrorProjects = [
project(':core-models'),
project(':dao-api'),
project(':dao-impl:ebean-dao'),
project(':dao-impl:elasticsearch-dao'),
// project(':dao-impl:neo4j-dao'),
project(':restli-resources'),
project(':testing'),
project(':validators')
]

allprojects {
apply plugin: 'idea'
apply plugin: 'eclipse'
apply plugin: 'checkstyle'

gradle.projectsEvaluated {
if (wErrorProjects.contains(project)) {
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror" <<
"-Xlint:-deprecation" << // TODO
"-Xlint:-processing" << // TODO we have annotations like @Nonnull that need a processor
"-Xlint:-serial" // I don't think we care about having custom Exception subclasses be serializable...
}
tasks.withType(JavaCompile) {
options.compilerArgs << "-Xlint:all" << "-Werror" <<
"-Xlint:-deprecation" << // TODO
"-Xlint:-processing" << // TODO we have annotations like @Nonnull that need a processor
"-Xlint:-serial" // I don't think we care about having custom Exception subclasses be serializable...
}
}
}
Expand Down
1 change: 0 additions & 1 deletion dao-api/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ apply from: "$rootDir/gradle/java-publishing.gradle"
dependencies {
compile project(':core-models')
compile project(':validators')
compile externalDependency.javatuples
compile externalDependency.reflections
compile externalDependency.commonsLang

Expand Down
44 changes: 32 additions & 12 deletions dao-api/src/main/java/com/linkedin/metadata/dao/BaseQueryDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
import java.util.List;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.javatuples.Triplet;
import lombok.AllArgsConstructor;
import lombok.Getter;

import static com.linkedin.metadata.dao.utils.QueryUtils.*;

Expand Down Expand Up @@ -112,28 +113,47 @@ List<RecordTemplate> findEntities(
@Nonnull Class<RELATIONSHIP> relationshipType, @Nonnull RelationshipFilter relationshipFilter, int minHops,
int maxHops, int offset, int count);

/**
* A path to visit.
*/
@Getter
@AllArgsConstructor
public static final class TraversalPath {
/**
* Relationship type on the traverse path.
*
* <p>Must be a type defined in com.linkedin.metadata.relationship.
*/
private final Class<? extends RecordTemplate> relationship;

/**
* The relationship filter to apply in a graph query.
*/
private final RelationshipFilter relationshipFilter;

/**
* Intermediate entity type on the traverse path.
*
* <p>Must be a type defined in com.linkedin.metadata.entity.
*/
private final Class<? extends RecordTemplate> intermediateEntity;
}

/**
* Finds a list of entities based on the given traversing paths.
*
* @param sourceEntityClass the source entity class as the starting point for the query
* @param sourceEntityClass the source entity class as the starting point for the query. Must be a type defined in
* com.linkedin.metadata.entity.
* @param sourceEntityFilter the filter to apply to the source entity when querying
* @param traversePaths specify the traverse paths via a list of (relationship type, relationship filter,
* intermediate entities)
* @param count the maximum number of entities to return. Ignored if set to a non-positive value.
*
* @param <SRC_ENTITY> source ENTITY type. Starting point of the traverse path. Must be a type defined in
* com.linkedin.metadata.entity.
* @param <INTER_ENTITY> intermediate entity type on the traverse path. Must be a type defined in
* com.linkedin.metadata.entity.
* @param <RELATIONSHIP> relationship type on the traverse path. Must be a type defined in
* com.linkedin.metadata.relationship.
* @return a list of entities that match the conditions specified in {@code filter}
*/
@Nonnull
public abstract <SRC_ENTITY extends RecordTemplate, RELATIONSHIP extends RecordTemplate, INTER_ENTITY extends RecordTemplate>
List<RecordTemplate> findEntities(
@Nullable Class<SRC_ENTITY> sourceEntityClass, @Nonnull Filter sourceEntityFilter,
@Nonnull List<Triplet<Class<RELATIONSHIP>, RelationshipFilter, Class<INTER_ENTITY>>> traversePaths, int offset, int count);
public abstract List<RecordTemplate> findEntities(@Nullable Class<? extends RecordTemplate> sourceEntityClass,
@Nonnull Filter sourceEntityFilter, @Nonnull List<TraversalPath> traversePaths, int offset, int count);

/**
* Finds a list of relationships of a specific type based on the given relationship filter and source entity filter.
Expand Down
21 changes: 10 additions & 11 deletions dao-impl/neo4j-dao/src/main/java/com/linkedin/metadata/dao/Neo4jQueryDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.util.stream.StreamSupport;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.javatuples.Triplet;
import org.neo4j.driver.Driver;
import org.neo4j.driver.Record;
import org.neo4j.driver.Session;
Expand Down Expand Up @@ -114,10 +113,8 @@ public <SRC_ENTITY extends RecordTemplate, DEST_ENTITY extends RecordTemplate, R

@Nonnull
@Override
public <SRC_ENTITY extends RecordTemplate, RELATIONSHIP extends RecordTemplate, INTER_ENTITY extends RecordTemplate> List<RecordTemplate> findEntities(
@Nullable Class<SRC_ENTITY> sourceEntityClass, @Nonnull Filter sourceEntityFilter,
@Nonnull List<Triplet<Class<RELATIONSHIP>, RelationshipFilter, Class<INTER_ENTITY>>> traversePaths, int offset,
int count) {
public List<RecordTemplate> findEntities(@Nullable Class<? extends RecordTemplate> sourceEntityClass,
@Nonnull Filter sourceEntityFilter, @Nonnull List<TraversalPath> traversePaths, int offset, int count) {
if (sourceEntityClass != null) {
EntityValidator.validateEntitySchema(sourceEntityClass);
}
Expand All @@ -128,16 +125,18 @@ public <SRC_ENTITY extends RecordTemplate, RELATIONSHIP extends RecordTemplate,
StringBuilder matchTemplate = new StringBuilder().append("MATCH (src%s %s)");
int pathCounter = 0;

// for each triplet, construct substatement via relationship type + relationship filer + inter entity
for (Triplet<Class<RELATIONSHIP>, RelationshipFilter, Class<INTER_ENTITY>> path : traversePaths) {
// for each path, construct substatement via relationship type + relationship filer + inter entity
for (TraversalPath path : traversePaths) {

pathCounter++; // Cannot use the same relationship variable 'r' or 'dest' for multiple patterns

final String edgeType = getTypeOrEmptyString(path.getValue0());
final String edgeCriteria = path.getValue1() == null ? "" : criterionToString(path.getValue1().getCriteria());
final String edgeType = getTypeOrEmptyString(path.getRelationship());
final String edgeCriteria =
path.getRelationshipFilter() == null ? "" : criterionToString(path.getRelationshipFilter().getCriteria());
final RelationshipDirection relationshipDirection =
path.getValue1() == null ? RelationshipDirection.UNDIRECTED : path.getValue1().getDirection();
final String destType = getTypeOrEmptyString(path.getValue2());
path.getRelationshipFilter() == null ? RelationshipDirection.UNDIRECTED
: path.getRelationshipFilter().getDirection();
final String destType = getTypeOrEmptyString(path.getIntermediateEntity());

String subTemplate = "-[r%d%s %s]-(dest%d%s)";
if (relationshipDirection == RelationshipDirection.INCOMING) {
Expand Down
65 changes: 30 additions & 35 deletions dao-impl/neo4j-dao/src/test/java/com/linkedin/metadata/dao/Neo4jQueryDAOTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.javatuples.Triplet;
import org.neo4j.driver.Driver;
import org.neo4j.driver.GraphDatabase;
import org.neo4j.driver.Record;
Expand Down Expand Up @@ -340,25 +339,21 @@ public void testFindEntitiesViaTraversePathes() throws Exception {
Filter sourceFilter = newFilter("urn", urn1.toString());

// use case: return all the datasets owned by corpgroup1
List paths = new ArrayList();
paths.add(
Triplet.with(RelationshipFoo.class, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.OUTGOING),
EntityBar.class));
paths.add(
Triplet.with(RelationshipBar.class, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.INCOMING),
EntityBaz.class));
List<BaseQueryDAO.TraversalPath> paths = new ArrayList<>();
paths.add(new BaseQueryDAO.TraversalPath(RelationshipFoo.class,
createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.OUTGOING), EntityBar.class));
paths.add(new BaseQueryDAO.TraversalPath(RelationshipBar.class,
createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.INCOMING), EntityBaz.class));
List<RecordTemplate> result = _dao.findEntities(EntityFoo.class, sourceFilter, paths, 0, 10);
assertEquals(result.size(), 1);
assertEquals(result.get(0), entity3);

// use case: return all the datasets owned by corpgroup1
List paths2 = new ArrayList();
paths2.add(
Triplet.with(RelationshipFoo.class, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.UNDIRECTED),
EntityBar.class));
paths2.add(
Triplet.with(RelationshipBar.class, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.UNDIRECTED),
EntityBaz.class));
List<BaseQueryDAO.TraversalPath> paths2 = new ArrayList<>();
paths2.add(new BaseQueryDAO.TraversalPath(RelationshipFoo.class,
createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.UNDIRECTED), EntityBar.class));
paths2.add(new BaseQueryDAO.TraversalPath(RelationshipBar.class,
createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.UNDIRECTED), EntityBaz.class));
List<RecordTemplate> result2 = _dao.findEntities(EntityFoo.class, sourceFilter, paths2, 0, 10);
assertEquals(result2, result);

Expand All @@ -379,38 +374,37 @@ public void testFindEntitiesViaTraversePathes() throws Exception {
RelationshipFoo relationshipFoo1To4 = new RelationshipFoo().setSource(urn1).setDestination(urn4);
_writer.addRelationship(relationshipFoo1To4);

List paths3 = new ArrayList();
paths3.add(
Triplet.with(RelationshipFoo.class, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.OUTGOING),
EntityBar.class));
paths3.add(
Triplet.with(RelationshipBar.class, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.INCOMING),
EntityBaz.class));
List<BaseQueryDAO.TraversalPath> paths3 = new ArrayList<>();
paths3.add(new BaseQueryDAO.TraversalPath(RelationshipFoo.class,
createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.OUTGOING), EntityBar.class));
paths3.add(new BaseQueryDAO.TraversalPath(RelationshipBar.class,
createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.INCOMING), EntityBaz.class));
List<RecordTemplate> result3 = _dao.findEntities(EntityFoo.class, sourceFilter, paths3, 0, 10);
assertEquals(result3.size(), 2);
assertEquals(result3.get(0), entity5);
assertEquals(result3.get(1), entity3);

// test nulls
List paths4 = new ArrayList();
paths4.add(Triplet.with(null, null, null));
List<BaseQueryDAO.TraversalPath> paths4 = new ArrayList<>();
paths4.add(new BaseQueryDAO.TraversalPath(null, null, null));
List<RecordTemplate> result4 = _dao.findEntities(EntityFoo.class, sourceFilter, paths4, 0, 10);
assertEquals(result4.size(), 2);
assertEquals(result4.get(0), entity4);
assertEquals(result4.get(1), entity2);

// test partial nulls with entity
List paths5 = new ArrayList();
List<BaseQueryDAO.TraversalPath> paths5 = new ArrayList<>();
paths5.add(
Triplet.with(null, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.OUTGOING), EntityBar.class));
new BaseQueryDAO.TraversalPath(null, createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.OUTGOING),
EntityBar.class));
List<RecordTemplate> result5 = _dao.findEntities(EntityFoo.class, sourceFilter, paths5, 0, 10);
assertEquals(result5.size(), 2);
assertEquals(result5.get(0), entity4);
assertEquals(result5.get(1), entity2);

// test partial nulls with relationship
List paths6 = new ArrayList();
paths6.add(Triplet.with(null, null, EntityBar.class));
List<BaseQueryDAO.TraversalPath> paths6 = new ArrayList<>();
paths6.add(new BaseQueryDAO.TraversalPath(null, null, EntityBar.class));
List<RecordTemplate> result6 = _dao.findEntities(EntityFoo.class, sourceFilter, paths6, 0, 10);
assertEquals(result6.size(), 2);
assertEquals(result6.get(0), entity4);
Expand Down Expand Up @@ -511,15 +505,17 @@ public void testFindNodesInPath() throws Exception {
// Get reports roll-up - 2 levels
Filter sourceFilter = newFilter("urn", urn1.toString());
RelationshipFilter relationshipFilter = createRelationshipFilter(EMPTY_FILTER, RelationshipDirection.INCOMING);
List<List<RecordTemplate>> paths = _dao.findPaths(EntityFoo.class, sourceFilter, null,
EMPTY_FILTER, RelationshipFoo.class, relationshipFilter, 1, 2, -1, -1);
List<List<RecordTemplate>> paths =
_dao.findPaths(EntityFoo.class, sourceFilter, null, EMPTY_FILTER, RelationshipFoo.class, relationshipFilter, 1,
2, -1, -1);
assertEquals(paths.size(), 5);
assertEquals(paths.stream().filter(l -> l.size() == 3).collect(Collectors.toList()).size(), 2);
assertEquals(paths.stream().filter(l -> l.size() == 5).collect(Collectors.toList()).size(), 3);

// Get reports roll-up - 1 level
paths = _dao.findPaths(EntityFoo.class, sourceFilter, null,
EMPTY_FILTER, RelationshipFoo.class, relationshipFilter, 1, 1, -1, -1);
paths =
_dao.findPaths(EntityFoo.class, sourceFilter, null, EMPTY_FILTER, RelationshipFoo.class, relationshipFilter, 1,
1, -1, -1);
assertEquals(paths.size(), 2);
assertEquals(paths.stream().filter(l -> l.size() == 3).collect(Collectors.toList()).size(), 2);
assertEquals(paths.stream().filter(l -> l.size() == 5).collect(Collectors.toList()).size(), 0);
Expand Down Expand Up @@ -601,9 +597,8 @@ public void testRunFreeFormQuery() throws Exception {

String cypherQuery = "MATCH (n {value:\"foo\"}) RETURN n ORDER BY n.urn";
List<Record> result = _dao.runFreeFormQuery(cypherQuery);
List<EntityFoo> nodes = result.stream()
.map(record -> _dao.nodeRecordToEntity(EntityFoo.class, record))
.collect(Collectors.toList());
List<EntityFoo> nodes =
result.stream().map(record -> _dao.nodeRecordToEntity(EntityFoo.class, record)).collect(Collectors.toList());
assertEquals(nodes.size(), 2);
assertEquals(nodes.get(0), entity1);
assertEquals(nodes.get(1), entity2);
Expand Down

0 comments on commit c5faa27

Please sign in to comment.