Format work requests according to ndjson spec (bazelbuild#14314)

wolfd · web-flow · commit 299e50aae9d8 · 2021-11-24T11:00:41.000+01:00
JSON persistent workers now delimit requests with newlines (`\n` on Unix, `\r\n` on Windows). Fixes bazelbuild#14218. Closes bazelbuild#14219. PiperOrigin-RevId: 411596359
diff --git a/src/main/java/com/google/devtools/build/lib/worker/JsonWorkerProtocol.java b/src/main/java/com/google/devtools/build/lib/worker/JsonWorkerProtocol.java
@@ -48,7 +48,10 @@ final class JsonWorkerProtocol implements WorkerProtocolImpl {
 
   @Override
   public void putRequest(WorkRequest request) throws IOException {
+    // WorkRequests are serialized according to ndjson spec.
+    // https://github.com/ndjson/ndjson-spec
     jsonPrinter.appendTo(request, jsonWriter);
+    jsonWriter.append(System.lineSeparator());
     jsonWriter.flush();
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerTest.java
@@ -118,12 +118,12 @@ public void testPutRequest_json_success() throws IOException, InterruptedExcepti
     testWorker.putRequest(WorkRequest.getDefaultInstance());
 
     OutputStream stdout = testWorker.getFakeSubprocess().getOutputStream();
-    assertThat(stdout.toString()).isEqualTo("{}");
+    assertThat(stdout.toString()).isEqualTo("{}" + System.lineSeparator());
   }
 
   @Test
   public void testGetResponse_json_success() throws IOException, InterruptedException {
-    TestWorker testWorker = createTestWorker("{}".getBytes(UTF_8), JSON);
+    TestWorker testWorker = createTestWorker(("{}" + System.lineSeparator()).getBytes(UTF_8), JSON);
     WorkResponse readResponse = testWorker.getResponse(0);
     WorkResponse response = WorkResponse.getDefaultInstance();
 
@@ -151,7 +151,8 @@ public void testPutRequest_json_populatedFields_success()
     OutputStream stdout = testWorker.getFakeSubprocess().getOutputStream();
     String requestJsonString =
         "{\"arguments\":[\"testRequest\"],\"inputs\":"
-            + "[{\"path\":\"testPath\",\"digest\":\"dGVzdERpZ2VzdA==\"}],\"requestId\":1,\"verbosity\":11}";
+            + "[{\"path\":\"testPath\",\"digest\":\"dGVzdERpZ2VzdA==\"}],\"requestId\":1,\"verbosity\":11}"
+            + System.lineSeparator();
     assertThat(stdout.toString()).isEqualTo(requestJsonString);
   }
 
@@ -170,7 +171,8 @@ public void testGetResponse_json_populatedFields_success()
 
   private void verifyGetResponseFailure(String responseString, String expectedError)
       throws IOException {
-    TestWorker testWorker = createTestWorker(responseString.getBytes(UTF_8), JSON);
+    TestWorker testWorker =
+        createTestWorker((responseString + System.lineSeparator()).getBytes(UTF_8), JSON);
     IOException ex = assertThrows(IOException.class, () -> testWorker.getResponse(0));
     assertThat(ex).hasMessageThat().contains(expectedError);
   }

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,10 @@ final class JsonWorkerProtocol implements WorkerProtocolImpl {`
`48`	`48`
`49`	`49`	`@Override`
`50`	`50`	`public void putRequest(WorkRequest request) throws IOException {`
	`51`	`+ // WorkRequests are serialized according to ndjson spec.`
	`52`	`+ // https://github.com/ndjson/ndjson-spec`
`51`	`53`	`jsonPrinter.appendTo(request, jsonWriter);`
	`54`	`+ jsonWriter.append(System.lineSeparator());`
`52`	`55`	`jsonWriter.flush();`
`53`	`56`	`}`
`54`	`57`