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.

Sign up for GitHub

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

fix: turn on werror for neo4j-dao #60

Merged
merged 1 commit into from
Dec 3, 2020
Merged

fix: turn on werror for neo4j-dao #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 5 additions & 21 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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 @@ -66,32 +65,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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