Skip to content

Commit

Permalink
Fix apoc.import.csv fail for NULL values when using INT datatype with…
Browse files Browse the repository at this point in the history 
… error [null] (#2156)

Fixes #2074

Co-authored-by: Giuseppe Villani <[email protected]>
  • Loading branch information
@github-actions @vga91
github-actions[bot] and vga91 authored Aug 11, 2021
1 parent 5b0c459 commit e94b519
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 10 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/apoc/export/csv/CsvEntityLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void loadNodes(final String fileName, final List<String> labels, final Gr
node.setProperty(field.getName(), idValue);
props++;
} else {
boolean propertyAdded = CsvPropertyConverter.addPropertyToGraphEntity(node, field, value);
boolean propertyAdded = CsvPropertyConverter.addPropertyToGraphEntity(node, field, value,clc);
props += propertyAdded ? 1 : 0;
}
}
Expand Down Expand Up @@ -232,7 +232,7 @@ public void loadRelationships(
for (CsvHeaderField field : edgePropertiesFields) {
final String name = field.getName();
Object value = result.map.get(name);
boolean propertyAdded = CsvPropertyConverter.addPropertyToGraphEntity(rel, field, value);
boolean propertyAdded = CsvPropertyConverter.addPropertyToGraphEntity(rel, field, value, clc);
props += propertyAdded ? 1 : 0;
}
reporter.update(0, 1, props);
Expand Down
16 changes: 15 additions & 1 deletion core/src/main/java/apoc/export/csv/CsvLoaderConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class CsvLoaderConfig {
private static final String SKIP_LINES = "skipLines";
private static final String BATCH_SIZE = "batchSize";
private static final String IGNORE_DUPLICATE_NODES = "ignoreDuplicateNodes";
private static final String IGNORE_BLANK_STRING = "ignoreBlankString";

private static char DELIMITER_DEFAULT = ',';
private static char ARRAY_DELIMITER_DEFAULT = ';';
Expand All @@ -23,6 +24,7 @@ public class CsvLoaderConfig {
private static int SKIP_LINES_DEFAULT = 1;
private static int BATCH_SIZE_DEFAULT = 2000;
private static boolean IGNORE_DUPLICATE_NODES_DEFAULT = false;
private static boolean IGNORE_BLANK_STRING_DEFAULT = false;

private final char delimiter;
private final char arrayDelimiter;
Expand All @@ -31,6 +33,7 @@ public class CsvLoaderConfig {
private final int skipLines;
private final int batchSize;
private final boolean ignoreDuplicateNodes;
private final boolean ignoreBlankString;

private CsvLoaderConfig(Builder builder) {
this.delimiter = builder.delimiter;
Expand All @@ -40,6 +43,7 @@ private CsvLoaderConfig(Builder builder) {
this.skipLines = builder.skipLines;
this.batchSize = builder.batchSize;
this.ignoreDuplicateNodes = builder.ignoreDuplicateNodes;
this.ignoreBlankString = builder.ignoreBlankString;
}

public char getDelimiter() {
Expand Down Expand Up @@ -68,6 +72,10 @@ public int getBatchSize() {

public boolean getIgnoreDuplicateNodes() { return ignoreDuplicateNodes; }

public boolean isIgnoreBlankString() {
return ignoreBlankString;
}

/**
* Creates builder to build {@link CsvLoaderConfig}.
*
Expand Down Expand Up @@ -102,7 +110,7 @@ public static CsvLoaderConfig from(Map<String, Object> config) {
if (config.get(SKIP_LINES) != null) builder.skipLines((int) config.get(SKIP_LINES));
if (config.get(BATCH_SIZE) != null) builder.batchSize((int) config.get(BATCH_SIZE));
if (config.get(IGNORE_DUPLICATE_NODES) != null) builder.ignoreDuplicateNodes((boolean) config.get(IGNORE_DUPLICATE_NODES));

if (config.get(IGNORE_BLANK_STRING) != null) builder.ignoreBlankString((boolean) config.get(IGNORE_BLANK_STRING));
return builder.build();
}

Expand All @@ -117,6 +125,7 @@ public static final class Builder {
private int skipLines = SKIP_LINES_DEFAULT;
private int batchSize = BATCH_SIZE_DEFAULT;
private boolean ignoreDuplicateNodes = IGNORE_DUPLICATE_NODES_DEFAULT;
private boolean ignoreBlankString = IGNORE_BLANK_STRING_DEFAULT;

private Builder() {
}
Expand Down Expand Up @@ -156,6 +165,11 @@ public Builder ignoreDuplicateNodes(boolean ignoreDuplicateNodes) {
return this;
}

public Builder ignoreBlankString(boolean ignoreBlankString) {
this.ignoreBlankString = ignoreBlankString;
return this;
}

public CsvLoaderConfig build() {
return new CsvLoaderConfig(this);
}
Expand Down
20 changes: 15 additions & 5 deletions core/src/main/java/apoc/export/csv/CsvPropertyConverter.java
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,40 @@
package apoc.export.csv;

import org.apache.commons.lang3.StringUtils;
import org.neo4j.graphdb.Entity;

import java.util.List;
import java.util.Objects;

public class CsvPropertyConverter {

public static boolean addPropertyToGraphEntity(Entity entity, CsvHeaderField field, Object value) {
if (field.isIgnore()) {
public static boolean addPropertyToGraphEntity(Entity entity, CsvHeaderField field, Object value, CsvLoaderConfig config) {
if (field.isIgnore() || value == null) {
return false;
}
if (field.isArray()) {
final List list = (List) value;
final boolean listContainingNull = list.stream().anyMatch(Objects::isNull);
if (listContainingNull) {
return false;
}
final Object[] prototype = getPrototypeFor(field.getType());
final Object[] array = ((List<Object>) value).toArray(prototype);
final Object[] array = list.toArray(prototype);
entity.setProperty(field.getName(), array);
} else {
if (config.isIgnoreBlankString() && value instanceof String && StringUtils.isBlank((String) value)) {
return false;
}
entity.setProperty(field.getName(), value);
}
return true;
}

static Object[] getPrototypeFor(String type) {
switch (type) {
case "INT": return new Integer [] {};
case "INT":
case "LONG": return new Long [] {};
case "FLOAT": return new Float [] {};
case "FLOAT":
case "DOUBLE": return new Double [] {};
case "BOOLEAN": return new Boolean [] {};
case "BYTE": return new Byte [] {};
Expand Down
40 changes: 39 additions & 1 deletion core/src/test/java/apoc/export/csv/ImportCsvTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,27 @@
import org.junit.Rule;
import org.junit.Test;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.QueryExecutionException;
import org.neo4j.internal.helpers.collection.Iterators;
import org.neo4j.test.rule.DbmsRule;
import org.neo4j.test.rule.ImpermanentDbmsRule;

import java.io.File;
import java.io.IOException;
import java.util.AbstractMap;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static apoc.util.MapUtil.map;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;

public class ImportCsvTest {
Expand Down Expand Up @@ -99,7 +105,13 @@ public class ImportCsvTest {
"2|Jane\n"),
new AbstractMap.SimpleEntry<>("personsWithoutIdField", "name:STRING\n" +
"John\n" +
"Jane\n")
"Jane\n"),
new AbstractMap.SimpleEntry<>("emptyInteger",
":ID(node_space_1),:LABEL,str_attribute:STRING,int_attribute:INT,int_attribute_array:INT[],double_attribute_array:FLOAT[]\n" +
"n1,Thing,once upon a time,1,\"2;3\",\"2.3;3.5\"\n" +
"n2,Thing,,2,\"4;5\",\"2.6;3.6\"\n" +
"n3,Thing,,,,\n"
)
).collect(Collectors.toMap(AbstractMap.SimpleEntry::getKey, AbstractMap.SimpleEntry::getValue)));

@Before
Expand Down Expand Up @@ -286,6 +298,32 @@ public void testCustomRelationshipTypes() {
Assert.assertEquals("John Jane", TestUtil.singleResultFirstColumn(db, "MATCH (p1:Person)-[:FRIENDS_WITH]->(p2:Person) RETURN p1.name + ' ' + p2.name AS pair ORDER BY pair"));
Assert.assertEquals("Jane John", TestUtil.singleResultFirstColumn(db, "MATCH (p1:Person)-[:KNOWS]->(p2:Person) RETURN p1.name + ' ' + p2.name AS pair ORDER BY pair"));
}

@Test
public void testEmptyInteger() {
TestUtil.testCall(db, "CALL apoc.import.csv([{fileName: 'file:/emptyInteger.csv', labels: ['entity']}], [], {ignoreBlankString: true})",
r -> assertEquals(3L, r.get("nodes")));

TestUtil.testResult(db, "MATCH (node:Thing) RETURN node ORDER BY node.int_attribute", r -> {
final Node firstNode = (Node) r.next().get("node");
final Map<String, Object> firstProps = firstNode.getAllProperties();
assertEquals(1L, firstProps.get("int_attribute"));
assertArrayEquals(new long[] { 2L, 3L }, (long[]) firstProps.get("int_attribute_array"));
assertArrayEquals(new double[] { 2.3D, 3.5D }, (double[]) firstProps.get("double_attribute_array"), 0);
final Node secondNode = (Node) r.next().get("node");
final Map<String, Object> secondProps = secondNode.getAllProperties();
assertEquals(2L, secondProps.get("int_attribute"));
assertArrayEquals(new long[] { 4L, 5L }, (long[]) secondProps.get("int_attribute_array"));
assertArrayEquals(new double[] { 2.6D, 3.6D }, (double[]) secondProps.get("double_attribute_array"), 0);
final Node thirdNode = (Node) r.next().get("node");
final Map<String, Object> thirdProps = thirdNode.getAllProperties();
assertNull(thirdProps.get("int_attribute"));
assertNull(thirdProps.get("int_attribute_array"));
assertNull(thirdProps.get("double_attribute_array"));
assertNull(thirdProps.get("str_attribute"));
assertFalse(r.hasNext());
});
}

@Test
public void testRelationshipWithoutIdSpaces() {
Expand Down
3 changes: 2 additions & 1 deletion docs/asciidoc/modules/ROOT/partials/usage/config/apoc.import.csv.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ The procedure support the following config parameters:
| quotationCharacter | String | " | quotation character | `--quote='"'`
| stringIds | Boolean | true | treat ids as strings | `--id-type=STRING`
| skipLines | Integer | 1 | lines to skip (incl. header) | N/A
|===
| ignoreBlankString | Boolean | false | if true ignore properties with a blank string | N/A
|===

0 comments on commit e94b519

Please sign in to comment.