From 3d8eb1b57da1b34b9e5993e1c728bd47530a3e06 Mon Sep 17 00:00:00 2001
From: Raz Crimson <52282402+RazCrimson@users.noreply.github.com>
Date: Sun, 12 Mar 2023 15:04:23 +0530
Subject: [PATCH] chg: Container.top to use stream_helper

fixes: Container.top
- infinite blocking when stream=True
- inconsistent decoding to json

add: test for Container.top with stream=True to avoid regression

Signed-off-by: Raz Crimson <52282402+RazCrimson@users.noreply.github.com>
---
 podman/domain/containers.py         | 16 ++++------
 podman/tests/unit/test_container.py | 47 +++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/podman/domain/containers.py b/podman/domain/containers.py
index 9354ab95..8236edf2 100644
--- a/podman/domain/containers.py
+++ b/podman/domain/containers.py
@@ -6,7 +6,6 @@
 from typing import Any, Dict, Iterable, Iterator, List, Mapping, Optional, Tuple, Union
 
 import requests
-from requests import Response
 
 from podman import api
 from podman.domain.images import Image
@@ -457,23 +456,20 @@ def top(self, **kwargs) -> Union[Iterator[Dict[str, Any]], Dict[str, Any]]:
             NotFound: when the container no longer exists
             APIError: when the service reports an error
         """
+        stream = kwargs.get("stream", False)
+
         params = {
+            "stream": stream,
             "ps_args": kwargs.get("ps_args"),
-            "stream": kwargs.get("stream", False),
         }
-        response = self.client.get(f"/containers/{self.id}/top", params=params)
+        response = self.client.get(f"/containers/{self.id}/top", params=params, stream=stream)
         response.raise_for_status()
 
-        if params["stream"]:
-            self._top_helper(response)
+        if stream:
+            return api.stream_helper(response, decode_to_json=True)
 
         return response.json()
 
-    @staticmethod
-    def _top_helper(response: Response) -> Iterator[Dict[str, Any]]:
-        for line in response.iter_lines():
-            yield line
-
     def unpause(self) -> None:
         """Unpause processes in container."""
         response = self.client.post(f"/containers/{self.id}/unpause")
diff --git a/podman/tests/unit/test_container.py b/podman/tests/unit/test_container.py
index f7294cf0..955f48c1 100644
--- a/podman/tests/unit/test_container.py
+++ b/podman/tests/unit/test_container.py
@@ -411,6 +411,53 @@ def test_top(self, mock):
         self.assertDictEqual(actual, body)
         self.assertTrue(adapter.called_once)
 
+    @requests_mock.Mocker()
+    def test_top_with_streaming(self, mock):
+        stream = [
+            {
+                "Processes": [
+                    [
+                        'jhonce',
+                        '2417',
+                        '2274',
+                        '0',
+                        'Mar01',
+                        '?',
+                        '00:00:01',
+                        (
+                            '/usr/bin/ssh-agent /bin/sh -c exec -l /bin/bash -c'
+                            ' "/usr/bin/gnome-session"'
+                        ),
+                    ],
+                    ['jhonce', '5544', '3522', '0', 'Mar01', 'pts/1', '00:00:02', '-bash'],
+                    ['jhonce', '6140', '3522', '0', 'Mar01', 'pts/2', '00:00:00', '-bash'],
+                ],
+                "Titles": ["UID", "PID", "PPID", "C", "STIME", "TTY", "TIME CMD"],
+            }
+        ]
+
+        buffer = io.StringIO()
+        for entry in stream:
+            buffer.write(json.JSONEncoder().encode(entry))
+            buffer.write("\n")
+
+        adapter = mock.get(
+            tests.LIBPOD_URL
+            + "/containers/87e1325c82424e49a00abdd4de08009eb76c7de8d228426a9b8af9318ced5ecd/top"
+            "?stream=True",
+            text=buffer.getvalue(),
+        )
+
+        container = Container(attrs=FIRST_CONTAINER, client=self.client.api)
+        top_stats = container.top(stream=True)
+
+        self.assertIsInstance(top_stats, Iterable)
+        for response, actual in zip(top_stats, stream):
+            self.assertIsInstance(response, dict)
+            self.assertDictEqual(response, actual)
+
+        self.assertTrue(adapter.called_once)
+
 
 if __name__ == '__main__':
     unittest.main()