Merge pull request #417 from sklump/fix-tails-file-download-hanging

use requests for now to fetch tails file reliably

NOTICES

@@ -374,6 +374,25 @@ Copyright (C) 2008-2011 INADA Naoki <[email protected]>
 -----------------------------------------------------------------
 -----------------------------------------------------------------
 
+=> requests
+
+Copyright (C) 2019 Kenneith Reitz
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+
+-----------------------------------------------------------------
+-----------------------------------------------------------------
+
 => prompt_toolkit
 
 Copyright (c) 2014, Jonathan Slenders

aries_cloudagent/revocation/models/revocation_registry.py

@@ -6,8 +6,10 @@
 from pathlib import Path
 from tempfile import gettempdir
 
+from requests import Session
+from requests.exceptions import RequestException
+
 from ...config.injection_context import InjectionContext
-from ...utils.http import FetchError, fetch_stream
 from ...utils.temp import get_temp_dir
 
 from ..error import RevocationError
@@ -163,11 +165,6 @@ async def retrieve_tails(self, context: InjectionContext):
             self.registry_id,
         )
 
-        try:
-            tails_stream = await fetch_stream(self._tails_public_uri)
-        except FetchError as e:
-            raise RevocationError("Error retrieving tails file") from e
-
         tails_file_path = Path(self.get_receiving_tails_local_path(context))
         tails_file_dir = tails_file_path.parent
         if not tails_file_dir.exists():
@@ -176,11 +173,15 @@ async def retrieve_tails(self, context: InjectionContext):
         buffer_size = 65536  # should be multiple of 32 bytes for sha256
         with open(tails_file_path, "wb", buffer_size) as tails_file:
             file_hasher = hashlib.sha256()
-            buf = await tails_stream.read(buffer_size)
-            while len(buf) > 0:
-                file_hasher.update(buf)
-                tails_file.write(buf)
-                buf = await tails_stream.read(buffer_size)
+
+            with Session() as req_session:
+                try:
+                    resp = req_session.get(self._tails_public_uri, stream=True)
+                    for buf in resp.iter_content(chunk_size=buffer_size):
+                        tails_file.write(buf)
+                        file_hasher.update(buf)
+                except RequestException as rx:
+                    raise RevocationError(f"Error retrieving tails file: {rx}")
 
             download_tails_hash = base58.b58encode(file_hasher.digest()).decode("utf-8")
             if download_tails_hash != self.tails_hash:

aries_cloudagent/revocation/models/tests/test_issuer_revocation_record.py

@@ -8,7 +8,7 @@
 from asynctest import TestCase as AsyncTestCase, mock as async_mock
 
 from ....config.injection_context import InjectionContext
-from ....issuer.base import BaseIssuer
+from ....issuer.base import BaseIssuer, IssuerError
 from ....issuer.indy import IndyIssuer
 from ....ledger.base import BaseLedger
 from ....storage.base import BaseStorage
@@ -50,6 +50,14 @@ async def test_generate_registry_etc(self):
         issuer = async_mock.MagicMock(BaseIssuer)
         self.context.injector.bind_instance(BaseIssuer, issuer)
 
+        with async_mock.patch.object(
+            issuer, "create_and_store_revocation_registry", async_mock.CoroutineMock()
+        ) as mock_create_store_rr:
+            mock_create_store_rr.side_effect = IssuerError("Not this time")
+
+            with self.assertRaises(RevocationError):
+                await rec.generate_registry(self.context, None)
+
         issuer.create_and_store_revocation_registry.return_value = (
             REV_REG_ID,
             json.dumps(

aries_cloudagent/revocation/models/tests/test_revocation_registry.py

@@ -90,13 +90,29 @@ async def test_properties(self):
         assert rev_reg.tails_local_path == "dummy"
         rev_reg.tails_public_uri = "dummy"
         assert rev_reg.tails_public_uri == "dummy"
+        return rev_reg.reg_def == REV_REG_DEF
 
     async def test_tails_local_path(self):
         rr_def_public = deepcopy(REV_REG_DEF)
         rr_def_public["value"]["tailsLocation"] = "http://sample.ca:8088/path"
-        rev_reg = RevocationRegistry.from_definition(rr_def_public, public_def=True)
+        rev_reg_pub = RevocationRegistry.from_definition(rr_def_public, public_def=True)
+
+        assert rev_reg_pub.get_receiving_tails_local_path(self.context) == TAILS_LOCAL
+
+        rev_reg_loc = RevocationRegistry.from_definition(REV_REG_DEF, public_def=False)
+        assert rev_reg_loc.get_receiving_tails_local_path(self.context) == TAILS_LOCAL
+
+        with async_mock.patch.object(
+            Path, "is_file", autospec=True
+        ) as mock_is_file:
+            mock_is_file.return_value = True
 
-        assert rev_reg.get_receiving_tails_local_path(self.context) == TAILS_LOCAL
+            assert await rev_reg_loc.get_or_fetch_local_tails_path(
+                self.context
+            ) == TAILS_LOCAL
+
+        rmtree(TAILS_DIR, ignore_errors=True)
+        assert not rev_reg_loc.has_local_tails_file(self.context)
 
     async def test_retrieve_tails(self):
         rev_reg = RevocationRegistry.from_definition(REV_REG_DEF, public_def=False)
@@ -108,39 +124,69 @@ async def test_retrieve_tails(self):
         rr_def_public["value"]["tailsLocation"] = "http://sample.ca:8088/path"
         rev_reg = RevocationRegistry.from_definition(rr_def_public, public_def=True)
 
+        more_magic = async_mock.MagicMock()
         with async_mock.patch.object(
-            test_module, "fetch_stream", async_mock.CoroutineMock()
-        ) as mock_fetch:
-            mock_fetch.side_effect = test_module.FetchError()
+            test_module, "Session", autospec=True
+        ) as mock_session:
+            mock_session.return_value.__enter__=async_mock.MagicMock(
+                return_value=more_magic
+            )
+            more_magic.get=async_mock.MagicMock(
+                side_effect=test_module.RequestException('Not this time')
+            )
 
             with self.assertRaises(RevocationError) as x_retrieve:
                 await rev_reg.retrieve_tails(self.context)
                 assert x_retrieve.message.contains("Error retrieving tails file")
 
-        rmtree(TAILS_DIR, ignore_errors=True)
+            rmtree(TAILS_DIR, ignore_errors=True)
 
+        more_magic = async_mock.MagicMock()
         with async_mock.patch.object(
-            test_module, "fetch_stream", async_mock.CoroutineMock()
-        ) as mock_fetch:
-            mock_fetch.return_value = async_mock.CoroutineMock(
-                read=async_mock.CoroutineMock(side_effect=[b"abcd1234", b""])
+            test_module, "Session", autospec=True
+        ) as mock_session:
+            mock_session.return_value.__enter__=async_mock.MagicMock(
+                return_value=more_magic
             )
+            more_magic.get=async_mock.MagicMock(
+                return_value=async_mock.MagicMock(
+                    iter_content=async_mock.MagicMock(
+                        side_effect=[(b"abcd1234",), (b"", )]
+                    )
+                )
+            )
+
             with self.assertRaises(RevocationError) as x_retrieve:
                 await rev_reg.retrieve_tails(self.context)
                 assert x_retrieve.message.contains(
-                    "The has of the downloaded tails file does not match."
+                    "The hash of the downloaded tails file does not match."
                 )
 
+            rmtree(TAILS_DIR, ignore_errors=True)
+
+        more_magic = async_mock.MagicMock()
         with async_mock.patch.object(
-            test_module, "fetch_stream", async_mock.CoroutineMock()
-        ) as mock_fetch, async_mock.patch.object(
+            test_module, "Session", autospec=True
+        ) as mock_session, async_mock.patch.object(
             base58, "b58encode", async_mock.MagicMock()
-        ) as mock_b58enc:
-            mock_fetch.return_value = async_mock.CoroutineMock(
-                read=async_mock.CoroutineMock(side_effect=[b"abcd1234", b""])
+        ) as mock_b58enc, async_mock.patch.object(
+            Path, "is_file", autospec=True
+        ) as mock_is_file:
+            mock_session.return_value.__enter__=async_mock.MagicMock(
+                return_value=more_magic
             )
+            more_magic.get=async_mock.MagicMock(
+                return_value=async_mock.MagicMock(
+                    iter_content=async_mock.MagicMock(
+                        side_effect=[(b"abcd1234",), (b"", )]
+                    )
+                )
+            )
+            mock_is_file.return_value = False
+
             mock_b58enc.return_value = async_mock.MagicMock(
                 decode=async_mock.MagicMock(return_value=TAILS_HASH)
             )
-            await rev_reg.retrieve_tails(self.context)
-            assert Path(TAILS_LOCAL).is_file()
+            await rev_reg.get_or_fetch_local_tails_path(self.context)
+
+            rmtree(TAILS_DIR, ignore_errors=True)

requirements.txt

@@ -9,3 +9,4 @@ marshmallow==3.0.0
 msgpack~=0.6.1
 prompt_toolkit~=2.0.9
 pynacl~=1.3.0
+requests~=2.23.0