Skip to content

Commit

Permalink
Split validator interface, implement name duplication validator
Browse files Browse the repository at this point in the history
  • Loading branch information
@fbiville
fbiville committed Feb 29, 2024
1 parent a1dd81c commit c5ba701
Show file tree
Hide file tree
Showing 6 changed files with 285 additions and 14 deletions.
6 changes: 2 additions & 4 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@
</developer>
</developers>
<properties>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
<maven.compiler.testSource>21</maven.compiler.testSource>
<maven.compiler.testTarget>21</maven.compiler.testTarget>
<maven.compiler.release>11</maven.compiler.release>
<maven.compiler.testRelease>21</maven.compiler.testRelease>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
<dependencyManagement>
Expand Down
49 changes: 42 additions & 7 deletions src/main/java/org/neo4j/importer/v1/ImportSpecificationDeserializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@
import com.networknt.schema.SpecVersion.VersionFlag;
import java.io.IOException;
import java.io.Reader;
import java.util.Iterator;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.ServiceLoader;
import org.neo4j.importer.v1.actions.Action;
import org.neo4j.importer.v1.validation.InvalidSpecificationException;
import org.neo4j.importer.v1.validation.SpecificationException;
import org.neo4j.importer.v1.validation.SpecificationValidationResult;
Expand Down Expand Up @@ -91,19 +94,51 @@ private static ImportSpecification deserialize(JsonNode json) throws Specificati
}

private static void runExtraValidations(ImportSpecification spec) throws SpecificationException {
Iterator<SpecificationValidator> validators =
ServiceLoader.load(SpecificationValidator.class).iterator();
Builder builder = SpecificationValidationResult.builder();
while (validators.hasNext()) {
SpecificationValidator validator = validators.next();
builder.merge(validator.validate(spec));
var validators = loadValidators();
var configuration =
spec.getConfiguration() == null ? Collections.<String, Object>emptyMap() : spec.getConfiguration();
validators.forEach(validator -> validator.visitConfiguration(configuration));
var sources = spec.getSources();
for (int i = 0; i < sources.size(); i++) {
final int index = i;
validators.forEach(validator -> validator.visitSource(index, sources.get(index)));
}
var targets = spec.getTargets();
var nodeTargets = targets.getNodes();
for (int i = 0; i < nodeTargets.size(); i++) {
final int index = i;
validators.forEach(validator -> validator.visitNodeTarget(index, nodeTargets.get(index)));
}
var relationshipTargets = targets.getRelationships();
for (int i = 0; i < relationshipTargets.size(); i++) {
final int index = i;
validators.forEach(validator -> validator.visitRelationshipTarget(index, relationshipTargets.get(index)));
}
var queryTargets = targets.getCustomQueries();
for (int i = 0; i < queryTargets.size(); i++) {
final int index = i;
validators.forEach(validator -> validator.visitCustomQueryTarget(index, queryTargets.get(index)));
}
var actions = spec.getActions() == null ? Collections.<Action>emptyList() : spec.getActions();
for (int i = 0; i < actions.size(); i++) {
final int index = i;
validators.forEach(validator -> validator.visitAction(index, actions.get(index)));
}

var builder = SpecificationValidationResult.builder();
validators.forEach(validator -> validator.accept(builder));
SpecificationValidationResult result = builder.build();
if (!result.passes()) {
throw new InvalidSpecificationException(result);
}
}

private static List<SpecificationValidator> loadValidators() {
List<SpecificationValidator> result = new ArrayList<>();
ServiceLoader.load(SpecificationValidator.class).forEach(result::add);
return result;
}

private static JsonNode parse(Reader spec) throws SpecificationException {
try {
return MAPPER.readTree(spec);
Expand Down
90 changes: 90 additions & 0 deletions src/main/java/org/neo4j/importer/v1/validation/NoDuplicatedNameValidator.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright (c) "Neo4j"
* Neo4j Sweden AB [http://neo4j.com]
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.neo4j.importer.v1.validation;

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import org.neo4j.importer.v1.actions.Action;
import org.neo4j.importer.v1.sources.Source;
import org.neo4j.importer.v1.targets.CustomQueryTarget;
import org.neo4j.importer.v1.targets.NodeTarget;
import org.neo4j.importer.v1.targets.RelationshipTarget;
import org.neo4j.importer.v1.validation.SpecificationValidationResult.Builder;

public class NoDuplicatedNameValidator implements SpecificationValidator {

private final NameCounter nameCounter;

public NoDuplicatedNameValidator() {
nameCounter = new NameCounter();
}

@Override
public void visitSource(int index, Source source) {
nameCounter.track(source.getName(), String.format("$.sources[%d]", index));
}

@Override
public void visitNodeTarget(int index, NodeTarget target) {
nameCounter.track(target.getName(), String.format("$.targets.nodes[%d]", index));
}

@Override
public void visitRelationshipTarget(int index, RelationshipTarget target) {
nameCounter.track(target.getName(), String.format("$.targets.relationships[%d]", index));
}

@Override
public void visitCustomQueryTarget(int index, CustomQueryTarget target) {
nameCounter.track(target.getName(), String.format("$.targets.queries[%d]", index));
}

@Override
public void visitAction(int index, Action action) {
nameCounter.track(action.getName(), String.format("$.actions[%d]", index));
}

@Override
public void accept(Builder builder) {
nameCounter.reportErrorsIfAny(builder);
}
}

class NameCounter {

private final Map<String, List<String>> pathsUsingName = new LinkedHashMap<>();

public void track(String name, String path) {
pathsUsingName.computeIfAbsent(name, (ignored) -> new ArrayList<>()).add(String.format("%s.name", path));
}

public void reportErrorsIfAny(Builder builder) {
pathsUsingName.entrySet().stream()
.filter(entry -> entry.getValue().size() > 1)
.forEach(entry -> {
List<String> paths = entry.getValue();
builder.addError(
paths.get(0),
"DUPN",
String.format(
"Name \"%s\" is duplicated across the following paths: %s",
entry.getKey(), String.join(", ", paths)));
});
}
}
22 changes: 19 additions & 3 deletions src/main/java/org/neo4j/importer/v1/validation/SpecificationValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,25 @@
*/
package org.neo4j.importer.v1.validation;

import org.neo4j.importer.v1.ImportSpecification;
import java.util.Map;
import java.util.function.Consumer;
import org.neo4j.importer.v1.actions.Action;
import org.neo4j.importer.v1.sources.Source;
import org.neo4j.importer.v1.targets.CustomQueryTarget;
import org.neo4j.importer.v1.targets.NodeTarget;
import org.neo4j.importer.v1.targets.RelationshipTarget;

public interface SpecificationValidator {
public interface SpecificationValidator extends Consumer<SpecificationValidationResult.Builder> {

SpecificationValidationResult validate(ImportSpecification specification);
default void visitConfiguration(Map<String, Object> configuration) {}

default void visitSource(int index, Source source) {}

default void visitNodeTarget(int index, NodeTarget target) {}

default void visitRelationshipTarget(int index, RelationshipTarget target) {}

default void visitCustomQueryTarget(int index, CustomQueryTarget target) {}

default void visitAction(int index, Action action) {}
}
1 change: 1 addition & 0 deletions src/main/resources/META-INF/services/org.neo4j.importer.v1.validation.SpecificationValidator
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.neo4j.importer.v1.validation.NoDuplicatedNameValidator
131 changes: 131 additions & 0 deletions src/test/java/org/neo4j/importer/v1/ImportSpecificationDeserializerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,4 +588,135 @@ void fails_if_name_is_blank_in_action() {
.hasMessageContainingAll(
"1 error(s)", "0 warning(s)", "$.actions[0].name: does not match the regex pattern \\S+");
}

@Test
void fails_if_source_name_is_duplicated_with_node_target() {
assertThatThrownBy(() -> deserialize(new StringReader(
"""
{
"sources": [{
"type": "bigquery",
"name": "duplicate",
"query": "SELECT id, name FROM my.table"
}],
"targets": {
"nodes": [{
"name": "duplicate",
"source": "duplicate",
"labels": ["Label1", "Label2"],
"write_mode": "create",
"properties": [
{"source_field": "field_1", "target_property": "property1"},
{"source_field": "field_2", "target_property": "property2"}
]
}]
}
}
"""
.stripIndent())))
.isInstanceOf(InvalidSpecificationException.class)
.hasMessageContainingAll(
"1 error(s)",
"0 warning(s)",
"Name \"duplicate\" is duplicated across the following paths: $.sources[0].name, $.targets.nodes[0].name");
}

@Test
void fails_if_source_name_is_duplicated_with_rel_target() {
assertThatThrownBy(() -> deserialize(new StringReader(
"""
{
"sources": [{
"type": "bigquery",
"name": "duplicate",
"query": "SELECT id, name FROM my.table"
}],
"targets": {
"relationships": [{
"name": "duplicate",
"source": "duplicate",
"type": "TYPE",
"start_node": {
"label": "Label1",
"key_properties": [
{"source_field": "field_1", "target_property": "property1"}
]
},
"end_node": {
"label": "Label2",
"key_properties": [
{"source_field": "field_2", "target_property": "property2"}
]
}
}]
}
}
"""
.stripIndent())))
.isInstanceOf(InvalidSpecificationException.class)
.hasMessageContainingAll(
"1 error(s)",
"0 warning(s)",
"Name \"duplicate\" is duplicated across the following paths: $.sources[0].name, $.targets.relationships[0].name");
}

@Test
void fails_if_source_name_is_duplicated_with_custom_query_target() {
assertThatThrownBy(() -> deserialize(new StringReader(
"""
{
"sources": [{
"type": "bigquery",
"name": "duplicate",
"query": "SELECT id, name FROM my.table"
}],
"targets": {
"queries": [{
"name": "duplicate",
"source": "duplicate",
"query": "UNWIND $rows AS row CREATE (n:ANode) SET n = row"
}]
}
}
"""
.stripIndent())))
.isInstanceOf(InvalidSpecificationException.class)
.hasMessageContainingAll(
"1 error(s)",
"0 warning(s)",
"Name \"duplicate\" is duplicated across the following paths: $.sources[0].name, $.targets.queries[0].name");
}

@Test
void fails_if_source_name_is_duplicated_with_action() {
assertThatThrownBy(() -> deserialize(new StringReader(
"""
{
"sources": [{
"type": "bigquery",
"name": "duplicate",
"query": "SELECT id, name FROM my.table"
}],
"targets": {
"queries": [{
"name": "my-target",
"source": "duplicate",
"query": "UNWIND $rows AS row CREATE (n:ANode) SET n = row"
}]
},
"actions": [{
"name": "duplicate",
"type": "http",
"method": "get",
"url": "https://example.com"
}]
}
"""
.stripIndent())))
.isInstanceOf(InvalidSpecificationException.class)
.hasMessageContainingAll(
"1 error(s)",
"0 warning(s)",
"Name \"duplicate\" is duplicated across the following paths: $.sources[0].name, $.actions[0].name");
}
}

0 comments on commit c5ba701

Please sign in to comment.