Fix JSON data response formatter to work well with empty data vs null in data (#20)

romkatsu · web-flow · commit 100f9755514c · 2020-11-03T00:02:46.000+03:00
diff --git a/src/DataResponse.php b/src/DataResponse.php
@@ -36,8 +36,8 @@ public function getBody(): StreamInterface
             return $this->dataStream;
         }
 
-        if (($response = $this->formatResponse()) !== null) {
-            $this->response = $response;
+        if ($this->hasResponseFormatter()) {
+            $this->response = $this->formatResponse();
             return $this->dataStream = $this->response->getBody();
         }
 
@@ -165,17 +165,22 @@ public function getData()
         return is_object($this->data) ? clone $this->data : $this->data;
     }
 
-    private function formatResponse(): ?ResponseInterface
+    public function hasData(): bool
     {
-        if ($this->responseFormatter !== null) {
-            $response = $this->responseFormatter->format($this);
-            if ($response instanceof self) {
-                throw new \RuntimeException('DataResponseFormatterInterface should not return instance of DataResponse.');
-            }
+        return $this->getData() !== null;
+    }
 
-            return $response;
-        } else {
-            return null;
+    /**
+     * @return ResponseInterface
+     * @psalm-suppress PossiblyNullReference
+     */
+    private function formatResponse(): ResponseInterface
+    {
+        $response = $this->responseFormatter->format($this);
+        if ($response instanceof self) {
+            throw new \RuntimeException('DataResponseFormatterInterface should not return instance of DataResponse.');
         }
+
+        return $response;
     }
 }
diff --git a/src/Formatter/JsonDataResponseFormatter.php b/src/Formatter/JsonDataResponseFormatter.php
@@ -14,6 +14,7 @@
 final class JsonDataResponseFormatter implements DataResponseFormatterInterface
 {
     use HasContentTypeTrait;
+
     /**
      * @var string the Content-Type header for the response
      */
@@ -23,8 +24,12 @@ final class JsonDataResponseFormatter implements DataResponseFormatterInterface
 
     public function format(DataResponse $dataResponse): ResponseInterface
     {
+        $content = '';
         $jsonSerializer = new JsonSerializer($this->options);
-        $content = $jsonSerializer->serialize($dataResponse->getData());
+        if ($dataResponse->hasData()) {
+            $content = $jsonSerializer->serialize($dataResponse->getData());
+        }
+
         $response = $dataResponse->getResponse();
         $response->getBody()->write($content);
 
diff --git a/tests/DataResponseTest.php b/tests/DataResponseTest.php
@@ -59,7 +59,7 @@ public function testSetEmptyResponseFormatter(): void
         $dataResponse->getBody()->rewind();
 
         $this->assertTrue($dataResponse->hasResponseFormatter());
-        $this->assertSame('null', $dataResponse->getBody()->getContents());
+        $this->assertSame('', $dataResponse->getBody()->getContents());
         $this->assertSame(['application/json'], $dataResponse->getHeader('Content-Type'));
     }
 
@@ -203,6 +203,24 @@ public function testGetData(): void
         $this->assertEquals('test2', $dataResponse->getData());
     }
 
+    public function testHasData(): void
+    {
+        $dataResponse = $this->createFactory()->createResponse('test');
+        $dataResponse = $dataResponse->withResponseFormatter(new JsonDataResponseFormatter());
+        $dataResponse->getBody()->rewind();
+
+        $this->assertTrue($dataResponse->hasData());
+    }
+
+    public function testHasDataWithEmptyData(): void
+    {
+        $dataResponse = $this->createFactory()->createResponse();
+        $dataResponse = $dataResponse->withResponseFormatter(new JsonDataResponseFormatter());
+        $dataResponse->getBody()->rewind();
+
+        $this->assertFalse($dataResponse->hasData());
+    }
+
     private function createFactory(): DataResponseFactory
     {
         return new DataResponseFactory(new Psr17Factory());
diff --git a/tests/Formatter/JsonDataResponseFormatterTest.php b/tests/Formatter/JsonDataResponseFormatterTest.php
@@ -42,6 +42,16 @@ public function testWithOptions(): void
         $this->assertSame(['application/json'], $result->getHeader(Header::CONTENT_TYPE));
     }
 
+    public function testWithEmptyResponse(): void
+    {
+        $dataResponse = $this->createFactory()->createResponse();
+        $result = (new JsonDataResponseFormatter())->withOptions(JSON_FORCE_OBJECT)->format($dataResponse);
+        $result->getBody()->rewind();
+
+        $this->assertSame('', $result->getBody()->getContents());
+        $this->assertSame(['application/json'], $result->getHeader(Header::CONTENT_TYPE));
+    }
+
     private function createFactory(): DataResponseFactory
     {
         return new DataResponseFactory(new Psr17Factory());

Original file line number	Diff line number	Diff line change
`@@ -36,8 +36,8 @@ public function getBody(): StreamInterface`
`36`	`36`	`return $this->dataStream;`
`37`	`37`	`}`
`38`	`38`
`39`		`- if (($response = $this->formatResponse()) !== null) {`
`40`		`- $this->response = $response;`
	`39`	`+ if ($this->hasResponseFormatter()) {`
	`40`	`+ $this->response = $this->formatResponse();`
`41`	`41`	`return $this->dataStream = $this->response->getBody();`
`42`	`42`	`}`
`43`	`43`
`@@ -165,17 +165,22 @@ public function getData()`
`165`	`165`	`return is_object($this->data) ? clone $this->data : $this->data;`
`166`	`166`	`}`
`167`	`167`
`168`		`- private function formatResponse(): ?ResponseInterface`
	`168`	`+ public function hasData(): bool`
`169`	`169`	`{`
`170`		`- if ($this->responseFormatter !== null) {`
`171`		`- $response = $this->responseFormatter->format($this);`
`172`		`- if ($response instanceof self) {`
`173`		`- throw new \RuntimeException('DataResponseFormatterInterface should not return instance of DataResponse.');`
`174`		`- }`
	`170`	`+ return $this->getData() !== null;`
	`171`	`+ }`
`175`	`172`
`176`		`- return $response;`
`177`		`- } else {`
`178`		`- return null;`
	`173`	`+ /**`
	`174`	`+ * @return ResponseInterface`
	`175`	`+ * @psalm-suppress PossiblyNullReference`
	`176`	`+ */`
	`177`	`+ private function formatResponse(): ResponseInterface`
	`178`	`+ {`
	`179`	`+ $response = $this->responseFormatter->format($this);`
	`180`	`+ if ($response instanceof self) {`
	`181`	`+ throw new \RuntimeException('DataResponseFormatterInterface should not return instance of DataResponse.');`
`179`	`182`	`}`
	`183`	`+`
	`184`	`+ return $response;`
`180`	`185`	`}`
`181`	`186`	`}`