Skip to content

Commit

Permalink
Removed unescaping of double quotes in CSV export, added tests in pri…
Browse files Browse the repository at this point in the history
…nter to ensure proper escape of adjacent double quotes & newlines
  • Loading branch information
xiazcy committed Nov 22, 2022
1 parent b03fcc1 commit 69b7fa9
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,7 @@ public static String escapeSeparators(Object value, String separator) {
}

public static String escapeDoubleQuotes(Object value) {
String temp = value.toString().replace("\"\"", "\"");
return temp.replace("\"", "\"\"");
return value.toString().replace("\"", "\"\"");
}

public String typeDescription() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,19 @@
import com.amazonaws.services.neptune.propertygraph.schema.DataType;
import com.amazonaws.services.neptune.propertygraph.schema.LabelSchema;
import com.amazonaws.services.neptune.propertygraph.schema.PropertySchema;
import org.apache.commons.csv.CSVFormat;
import org.apache.commons.csv.CSVRecord;
import org.junit.Test;

import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.stream.Collectors;

import static org.junit.Assert.assertEquals;

Expand Down Expand Up @@ -112,4 +119,112 @@ public void shouldUseEmptySeparatorToSeparateMultipleValues() throws Exception {
stringWriter.toString());
}

@Test
public void shouldEscapeTwoDoubleQuoteAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("{\"hobby\" : \"watching \"Flash\"\"}",
"\"{\"\"hobby\"\" : \"\"watching \"\"Flash\"\"\"\"}\"",
new PrinterOptions(CsvPrinterOptions.builder().build()));
}

@Test
public void shouldEscapeThreeDoubleQuoteAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("{\"hobby\" : \"watching \"The \"Flash\"\"\"}",
"\"{\"\"hobby\"\" : \"\"watching \"\"The \"\"Flash\"\"\"\"\"\"}\"",
new PrinterOptions(CsvPrinterOptions.builder().build()));
}

@Test
public void shouldPrintCommaInStringWhenPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("{\"hobby\", \"watching \"The \"Flash\"\"}",
"\"{\"\"hobby\"\", \"\"watching \"\"The \"\"Flash\"\"\"\"}\"",
new PrinterOptions(CsvPrinterOptions.builder().build()));
}

@Test
public void shouldNotEscapeNewlineCharAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("A\nB", "\"A\nB\"",
new PrinterOptions(CsvPrinterOptions.builder().build()));
}

@Test
public void shouldNotEscapeNewlineAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("A" + System.lineSeparator() + "B", "\"A\nB\"",
new PrinterOptions(CsvPrinterOptions.builder().build()));
}

@Test
public void shouldEscapeNewlineCharSetTrueAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("A\nB",
"\"A\\nB\"",
new PrinterOptions(CsvPrinterOptions.builder().setEscapeNewline(true).build()));
}

@Test
public void shouldEscapeNewlineSetTrueAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("A" + System.lineSeparator() + "B",
"\"A\\nB\"",
new PrinterOptions(CsvPrinterOptions.builder().setEscapeNewline(true).build()));
}

@Test
public void shouldNotEscapeNewlineCharsAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("A\n\nB", "\"A\n\nB\"",
new PrinterOptions(CsvPrinterOptions.builder().build()));
}

@Test
public void shouldEscapeNewlineCharsSetTrueAfterPrintPropertiesToCSVAndRewrite() throws Exception {
testEscapeCharacterAfterPrintPropertiesAndRewrite("A\n\nB",
"\"A\\n\\nB\"",
new PrinterOptions(CsvPrinterOptions.builder().setEscapeNewline(true).build()));
}

// A set of tests to ensure that String escaping is done properly when CSVPropertyGraphPrinter prints to
// a buffer, so when the buffer is read in by CSVFormat, the original property string is received
private void testEscapeCharacterAfterPrintPropertiesAndRewrite(String originalValue, String expectedValue, PrinterOptions printerOptions) throws IOException {
StringWriter stringWriter = new StringWriter();

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

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

HashMap<String, List<String>> props = new HashMap<String, List<String>>() {{
put("property1", Collections.singletonList(originalValue));
}};

CsvPropertyGraphPrinter printer = new CsvPropertyGraphPrinter(
new PrintOutputWriter("outputId", stringWriter),
labelSchema,
printerOptions);

printer.printProperties(props);

// all double quotes should be escaped when printer prints
assertEquals(expectedValue, stringWriter.toString());

// using CSVFormat to read in printed items (same library used by RewriteCSV)
String[] filePropertyHeaders = labelSchema.propertySchemas().stream()
.map(p -> p.property().toString())
.collect(Collectors.toList())
.toArray(new String[]{});
CSVFormat format = CSVFormat.RFC4180.withHeader(filePropertyHeaders);
Reader in = new StringReader(stringWriter.toString());
Iterable<CSVRecord> records = format.parse(in);

for (CSVRecord record : records) {
// what CSVFormat read in from printed CSV should be the original value
if (printerOptions.csv().escapeNewline()){
// parsed record will contain escaped newline, to compare to original we have to unescape it
assertEquals(originalValue, record.get("property1").replace("\\n", "\n"));
} else {
assertEquals(originalValue, record.get("property1"));
}

// double quotes should all be properly escaped again when we format for rewrite
assertEquals(expectedValue, DataType.String.format(record.get("property1"), printerOptions.csv().escapeNewline()));
}
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,15 @@ public void shouldEscapeDoubleQuotes() {
}

@Test
public void shouldNotDoubleEscapeDoubleQuotesThatHaveAlreadyBeenEscaped() {
public void shouldEscapeTwoDoubleQuotes() {
String result = DataType.String.format("One \"\"two\"\" three");
assertEquals("\"One \"\"two\"\" three\"", result);
assertEquals("\"One \"\"\"\"two\"\"\"\" three\"", result);
}

@Test
public void shouldEscapeThreeDoubleQuotes() {
String result = DataType.String.format("One \"\"\"two\"\"\" three");
assertEquals("\"One \"\"\"\"\"\"two\"\"\"\"\"\" three\"", result);
}

@Test
Expand Down

0 comments on commit 69b7fa9

Please sign in to comment.