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

Jump to bottom

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82f74d2
Make neptune-export cross-platform by removing hardcoded line and pat…
Feb 10, 2022
728aed9
print nested properties map as nested json object instead of string
Mar 2, 2022
acbbe1a
make strictCardinality configurable at command level
Mar 3, 2022
dce137c
Merge pull request #1 from drossi750/task/handle_nested_properties_ma…
drossi750 Mar 11, 2022
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
6 changes: 5 additions & 1 deletion ...t/src/main/java/com/amazonaws/services/neptune/ExportPropertyGraphFromGremlinQueries.java
View file
Open in desktop
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
27 changes: 23 additions & 4 deletions ...c/main/java/com/amazonaws/services/neptune/propertygraph/io/JsonPropertyGraphPrinter.java
View file
Open in desktop
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<?, ?>;
}
}
2 changes: 1 addition & 1 deletion ...ne-export/src/main/java/com/amazonaws/services/neptune/propertygraph/schema/DataType.java
View file
Open in desktop
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
12 changes: 4 additions & 8 deletions neptune-export/src/main/java/com/amazonaws/services/neptune/util/S3ObjectInfo.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion neptune-export/src/test/java/com/amazonaws/services/neptune/io/DirectoriesTest.java
View file
Open in desktop
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
32 changes: 28 additions & 4 deletions ...st/java/com/amazonaws/services/neptune/propertygraph/io/JsonPropertyGraphPrinterTest.java
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions ...m/amazonaws/services/neptune/propertygraph/io/VariableRowCsvPropertyGraphPrinterTest.java
View file
Open in desktop
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
9 changes: 6 additions & 3 deletions ...xport/src/test/java/com/amazonaws/services/neptune/propertygraph/schema/DataTypeTest.java
View file
Open in desktop
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
10 changes: 8 additions & 2 deletions neptune-export/src/test/java/com/amazonaws/services/neptune/util/S3ObjectInfoTest.java
View file
Open in desktop
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