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.

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

Make neptune-export cross-platform by removing hardcoded line and path separators #202

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ public class ExportPropertyGraphFromGremlinQueries extends NeptuneExportCommand
@Once
private boolean includeTypeDefinitions = false;

@Option(name = {"--strict-cardinality"}, description = "Use strict cardinality (optional, default 'false').")
@Once
private boolean strictCardinality = false;

@Option(name = {"--timeout-millis"}, description = "Query timeout in milliseconds (optional).")
@Once
private Long timeoutMillis = null;
Expand All @@ -94,7 +98,7 @@ public void run() {
directories.queriesResource();

CsvPrinterOptions csvPrinterOptions = CsvPrinterOptions.builder().setIncludeTypeDefinitions(includeTypeDefinitions).build();
JsonPrinterOptions jsonPrinterOptions = JsonPrinterOptions.builder().setStrictCardinality(true).build();
JsonPrinterOptions jsonPrinterOptions = JsonPrinterOptions.builder().setStrictCardinality(strictCardinality).build();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change to the default behavior of setStrictCardinality(true). Instead of changing the default behavior, can we leave strictCardinality as true and change the parameter to accept --strict-cardinality false instead?


PropertyGraphTargetConfig targetConfig = target.config(directories, new PrinterOptions(csvPrinterOptions, jsonPrinterOptions));
NamedQueriesCollection namedQueries = getNamedQueriesCollection(queries, queriesFile, queriesResource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,27 @@ public void printProperties(Map<?, ?> properties) throws IOException {
}

private void printProperty(Object value, PropertySchema propertySchema) throws IOException {

DataType dataType = propertySchema.dataType();
String formattedKey = propertySchema.nameWithoutDataType();
boolean isMultiValue = propertySchema.isMultiValue();

printProperty(value, dataType, formattedKey, isMultiValue);
if (isMap(value)) {
generator.writeFieldName(formattedKey);
printStartRow();
printNestedProperties((Map<?, ?>) value);
printEndRow();
} else {
DataType dataType = propertySchema.dataType();
boolean isMultiValue = propertySchema.isMultiValue();

printProperty(value, dataType, formattedKey, isMultiValue);
}
}

private void printNestedProperties(Map<?, ?> value) throws IOException {
for (Map.Entry<?, ?> property : value.entrySet()) {
PropertySchema propertySchema = new PropertySchema(property.getKey());
propertySchema.accept(property.getValue(), true);
printProperty(property.getValue(), propertySchema);
}
}

private void printProperty(Object value, DataType dataType, String formattedKey, boolean forceMultiValue) throws IOException {
Expand Down Expand Up @@ -210,4 +225,8 @@ public void close() throws Exception {
private boolean isList(Object value) {
return value instanceof List<?>;
}

private boolean isMap(Object value) {
return value instanceof Map<?, ?>;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ public String format(Object value, boolean escapeNewline) {
}

private String escapeNewlineChar(String value) {
return value.replace("\n", "\\n");
return value.replace("\r", "\\r").replace("\n", "\\n");
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,13 @@ public File createDownloadFile(String parent) {
}

public S3ObjectInfo withNewKeySuffix(String suffix) {
File file = StringUtils.isNotEmpty(key) ? new File(key, suffix) : new File(suffix);
return new S3ObjectInfo( String.format("s3://%s/%s", bucket, file.getPath()));
String newKey = StringUtils.isNotEmpty(key) ? key.replaceFirst("([^/])$","$1/") + suffix : suffix;
return new S3ObjectInfo( String.format("s3://%s/%s", bucket, newKey));
}

public S3ObjectInfo replaceOrAppendKey(String placeholder, String ifPresent, String ifAbsent) {

File file = key.contains(placeholder) ?
new File(key.replace(placeholder, ifPresent)) :
new File(key, ifAbsent);

return new S3ObjectInfo( String.format("s3://%s/%s", bucket, file.getPath()));
String finalKey = key.contains(placeholder) ? key.replace(placeholder, ifPresent) : key.replaceFirst("([^/])$|^$","$1/") + ifAbsent;
return new S3ObjectInfo( String.format("s3://%s/%s", bucket, finalKey));
}

public S3ObjectInfo replaceOrAppendKey(String placeholder, String ifPresent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void createsDigestFilePathsForVeryLongFilenames() throws IOException {
Directories directories = Directories.createFor(DirectoryStructure.PropertyGraph, new File("home"), "export-id", "", "");
Path filePath = directories.createFilePath(path, longName, PropertyGraphExportFormat.csv);

assertEquals("/export/8044f12c352773b7ff400ef524da6e90db419e4a.csv", filePath.toString());
assertEquals(File.separator + "export" + File.separator + "8044f12c352773b7ff400ef524da6e90db419e4a.csv", filePath.toString());
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,30 @@ public void shouldPrintMultiValueListAsArrayIrrespectiveOfWhetherMultiValueIsTru
stringWriter.toString());
}

@Test
public void shouldPrintNestedPropertiesMapAsJsonObject() throws Exception {
StringWriter stringWriter = new StringWriter();

PropertySchema propertySchema1 = new PropertySchema("property", false, DataType.String, true);

LabelSchema labelSchema = new LabelSchema(new Label("Entity"));
labelSchema.put("property", propertySchema1);

Map<?, ?> props = map(entry("property", map(
entry("nestedProperty1", "value1"),
entry("nestedProperty2", "value2"))));

try (PropertyGraphPrinter propertyGraphPrinter = PropertyGraphExportFormat.json.createPrinter(new PrintOutputWriter("outputId", stringWriter), labelSchema, PrinterOptions.NULL_OPTIONS)) {
propertyGraphPrinter.printStartRow();
propertyGraphPrinter.printProperties(props);
propertyGraphPrinter.printEndRow();
}

assertEquals(
"{\"property\":{\"nestedProperty1\":\"value1\",\"nestedProperty2\":\"value2\"}}",
stringWriter.toString());
}

@Test
public void appendsPreviouslyUnseenValuesToObjectWhenInferringSchema() throws IOException {

Expand All @@ -197,10 +221,10 @@ public void appendsPreviouslyUnseenValuesToObjectWhenInferringSchema() throws IO
map(entry("fname", "fname5"), entry("lname", "lname5"), entry("age", 50))
);

String expectedOutput = "{\"fname\":\"fname1\"}\n" +
"{\"fname\":\"fname2\",\"lname\":\"lname2\"}\n" +
"{\"fname\":\"fname3\",\"age\":30}\n" +
"{\"lname\":\"lname4\",\"age\":40}\n" +
String expectedOutput = "{\"fname\":\"fname1\"}" + System.lineSeparator() +
"{\"fname\":\"fname2\",\"lname\":\"lname2\"}" + System.lineSeparator() +
"{\"fname\":\"fname3\",\"age\":30}" + System.lineSeparator() +
"{\"lname\":\"lname4\",\"age\":40}" + System.lineSeparator() +
"{\"fname\":\"fname5\",\"lname\":\"lname5\",\"age\":50}";

assertEquals(expectedOutput, stringWriter.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ public void appendsPreviouslyUnseenColumnsToEndOfRow() throws IOException {
map(entry("fname", "fname5"), entry("lname", "lname5"), entry("age", 50))
);

String expectedOutput = "\"fname1\"\n" +
"\"fname2\",\"lname2\"\n" +
"\"fname3\",,30\n" +
",\"lname4\",40\n" +
"\"fname5\",\"lname5\",50\n";
String expectedOutput = "\"fname1\"" + System.lineSeparator() +
"\"fname2\",\"lname2\"" + System.lineSeparator() +
"\"fname3\",,30" + System.lineSeparator() +
",\"lname4\",40" + System.lineSeparator() +
"\"fname5\",\"lname5\",50" + System.lineSeparator();

assertEquals(expectedOutput, stringWriter.toString());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void shouldNotEscapeNewlineChar(){
@Test
public void shouldNotEscapeNewline(){
String result = DataType.String.format("A" + System.lineSeparator() + "B");
assertEquals("\"A\nB\"", result);
assertEquals("\"A" + System.lineSeparator() + "B\"", result);
}

@Test
Expand All @@ -82,8 +82,11 @@ public void shouldEscapeNewlineCharIfEscapeNewlineSetToTrue(){

@Test
public void shouldEscapeNewlineIfEscapeNewlineSetToTrue(){
String result = DataType.String.format("A" + System.lineSeparator() + "B", true);
assertEquals("\"A\\nB\"", result);
String result1 = DataType.String.format("A\r\nB", true);
assertEquals("\"A\\r\\nB\"", result1);

String result2 = DataType.String.format("A\nB", true);
assertEquals("\"A\\nB\"", result2);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import org.junit.Test;

import java.io.File;

import static org.junit.Assert.assertEquals;

public class S3ObjectInfoTest {
Expand Down Expand Up @@ -51,7 +53,9 @@ public void canCreateDownloadFileForKeyWithoutTrailingSlash(){

S3ObjectInfo s3ObjectInfo = new S3ObjectInfo(s3Uri);

assertEquals("/temp/c.txt", s3ObjectInfo.createDownloadFile("/temp").getAbsolutePath());
String parent = new File(System.getProperty("java.io.tmpdir")).getAbsolutePath();
String absolutePath = s3ObjectInfo.createDownloadFile(parent).getAbsolutePath();
assertEquals(parent + File.separator + "c.txt", absolutePath);
}

@Test
Expand All @@ -60,7 +64,9 @@ public void canCreateDownloadFileForKeyWithTrailingSlash(){

S3ObjectInfo s3ObjectInfo = new S3ObjectInfo(s3Uri);

assertEquals("/temp/c", s3ObjectInfo.createDownloadFile("/temp").getAbsolutePath());
String parent = new File(System.getProperty("java.io.tmpdir")).getAbsolutePath();
String absolutePath = s3ObjectInfo.createDownloadFile(parent).getAbsolutePath();
assertEquals(parent + File.separator + "c", absolutePath);
}

@Test
Expand Down