fabriziofortino commented on code in PR #964:
URL: https://github.com/apache/jackrabbit-oak/pull/964#discussion_r1222994405


##########
oak-store-spi/src/test/java/org/apache/jackrabbit/oak/json/JsonSerializerTest.java:
##########
@@ -63,13 +126,59 @@ public void childOrder() throws Exception{
             }
 
         } while (reader.matches(','));
+        Assert.assertTrue(customLogs.getLogs().stream().filter(n -> 
n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
+        assertEquals(expectedOrder, childNames);
+    }
+
+    @Test
+    public void jsonFlushedInChunks() throws Exception {
+        System.setProperty(JsonSerializer.WRITER_FLUSH_THRESHOLD_KEY, "" + 10);
+        NodeBuilder builder = EMPTY_NODE.builder();
+        builder.child("a");
+        builder.child("b");
+        builder.child("c");
+        builder.child("d");
+
+        List<String> expectedOrder = Arrays.asList("a", "c", "b", "d");
+        builder.setProperty(":childOrder", expectedOrder, Type.NAMES);
+
+        NodeState state = builder.getNodeState();
+        String json = serializeJsonFlushedInChunks(state);
 
+        JsopReader reader = new JsopTokenizer(json);
+        List<String> childNames = Lists.newArrayList();
+        reader.read('{');
+        do {
+            String key = reader.readString();
+            reader.read(':');
+            if (reader.matches('{')) {
+                childNames.add(key);
+                reader.matches('}');
+            }
+
+        } while (reader.matches(','));
+        Assert.assertFalse(customLogs.getLogs().stream().filter(n -> 
n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());

Review Comment:
   same here
   ```suggestion
           Assert.assertFalse(customLogs.getLogs().stream().anyMatch(n -> 
n.contains("Json is not getting flushed in chunks:")));```



##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonSerializer.java:
##########
@@ -79,17 +79,30 @@ public class JsonSerializer {
     private final BlobSerializer blobs;
 
     private final boolean catchExceptions;
+    private final PrintWriter printWriter;
+    public static final String WRITER_FLUSH_THRESHOLD_KEY = 
"oak.writerFlushThreshold";
+
+    private final long defaultWriterFlushThreshold = 100*1024*1024L; // 100 MB

Review Comment:
   minor: improve code formatting
   ```suggestion
       private final long defaultWriterFlushThreshold = 100 * 1024 * 1024L; // 
100 MB
   ```



##########
oak-store-spi/src/main/java/org/apache/jackrabbit/oak/json/JsonSerializer.java:
##########
@@ -184,6 +202,13 @@ public void serialize(NodeState node, String basePath) {
         }
 
         json.endObject();
+        if (printWriter != null && json instanceof JsopBuilder && 
((JsopBuilder) json).length() >= writerFlushThresholdBytes) {
+            printWriter.print(json);
+            printWriter.flush();
+            ((JsopBuilder) json).flushWriter();
+        } else if (printWriter == null || !(json instanceof JsopBuilder)) {

Review Comment:
   should we remove the `if` and leave just the `else`?



##########
oak-store-spi/src/test/java/org/apache/jackrabbit/oak/json/JsonSerializerTest.java:
##########
@@ -63,13 +126,59 @@ public void childOrder() throws Exception{
             }
 
         } while (reader.matches(','));
+        Assert.assertTrue(customLogs.getLogs().stream().filter(n -> 
n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());

Review Comment:
   this could be simplified
   ```suggestion
           Assert.assertTrue(customLogs.getLogs().stream().anyMatch(n -> 
n.contains("Json is not getting flushed in chunks:")));
   ```



##########
oak-store-spi/src/test/java/org/apache/jackrabbit/oak/json/JsonSerializerTest.java:
##########
@@ -63,13 +126,59 @@ public void childOrder() throws Exception{
             }
 
         } while (reader.matches(','));
+        Assert.assertTrue(customLogs.getLogs().stream().filter(n -> 
n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
+        assertEquals(expectedOrder, childNames);
+    }
+
+    @Test
+    public void jsonFlushedInChunks() throws Exception {
+        System.setProperty(JsonSerializer.WRITER_FLUSH_THRESHOLD_KEY, "" + 10);
+        NodeBuilder builder = EMPTY_NODE.builder();
+        builder.child("a");
+        builder.child("b");
+        builder.child("c");
+        builder.child("d");
+
+        List<String> expectedOrder = Arrays.asList("a", "c", "b", "d");
+        builder.setProperty(":childOrder", expectedOrder, Type.NAMES);
+
+        NodeState state = builder.getNodeState();
+        String json = serializeJsonFlushedInChunks(state);
 
+        JsopReader reader = new JsopTokenizer(json);
+        List<String> childNames = Lists.newArrayList();
+        reader.read('{');
+        do {
+            String key = reader.readString();
+            reader.read(':');
+            if (reader.matches('{')) {
+                childNames.add(key);
+                reader.matches('}');
+            }
+
+        } while (reader.matches(','));
+        Assert.assertFalse(customLogs.getLogs().stream().filter(n -> 
n.contains("Json is not getting flushed in chunks:")).findFirst().isPresent());
         assertEquals(expectedOrder, childNames);
     }
 
-    private String serialize(NodeState nodeState){
+    private String serialize(NodeState nodeState) {
         JsopBuilder json = new JsopBuilder();
         new JsonSerializer(json, "{\"properties\":[\"*\", \"-:*\"]}", new 
BlobSerializer()).serialize(nodeState);
         return json.toString();
     }
+
+    private String serializeJsonFlushedInChunks(NodeState nodeState) throws 
IOException {
+        PrintWriter pw;
+        OutputStream writerStream;
+        File outFile = temporaryFolder.newFile();
+        OutputStream os = new 
BufferedOutputStream(FileUtils.openOutputStream(outFile));
+        writerStream = os;
+        pw = new PrintWriter(writerStream);

Review Comment:
   the `os` variable is redundant. Make it more concise
   ```suggestion
           File outFile = temporaryFolder.newFile();
           OutputStream writerStream = new 
BufferedOutputStream(FileUtils.openOutputStream(outFile));
           PrintWriter pw = new PrintWriter(writerStream);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to