From 078622cfb0ee364acba5d91a20244f9c1ee87d30 Mon Sep 17 00:00:00 2001
From: Maxime Nannan <28675918+mnannan@users.noreply.github.com>
Date: Fri, 20 May 2022 08:30:06 +0200
Subject: [PATCH] Fix file expiration issue with GCS (#5318)

---
 scrapy/pipelines/files.py    | 10 +++++++---
 tests/test_pipeline_files.py | 23 +++++++++++++++++++++++
 tox.ini                      |  7 ++++---
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index 5c52c6c28..906e7eb24 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -222,8 +222,8 @@ class GCSFilesStore:
                 return {'checksum': checksum, 'last_modified': last_modified}
             else:
                 return {}
-
-        return threads.deferToThread(self.bucket.get_blob, path).addCallback(_onsuccess)
+        blob_path = self._get_blob_path(path)
+        return threads.deferToThread(self.bucket.get_blob, blob_path).addCallback(_onsuccess)
 
     def _get_content_type(self, headers):
         if headers and 'Content-Type' in headers:
@@ -231,8 +231,12 @@ class GCSFilesStore:
         else:
             return 'application/octet-stream'
 
+    def _get_blob_path(self, path):
+        return self.prefix + path
+
     def persist_file(self, path, buf, info, meta=None, headers=None):
-        blob = self.bucket.blob(self.prefix + path)
+        blob_path = self._get_blob_path(path)
+        blob = self.bucket.blob(blob_path)
         blob.cache_control = self.CACHE_CONTROL
         blob.metadata = {k: str(v) for k, v in (meta or {}).items()}
         return threads.deferToThread(
diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py
index 4e1b90787..0ff2045ed 100644
--- a/tests/test_pipeline_files.py
+++ b/tests/test_pipeline_files.py
@@ -525,6 +525,29 @@ class TestGCSFilesStore(unittest.TestCase):
         self.assertEqual(blob.content_type, 'application/octet-stream')
         self.assertIn(expected_policy, acl)
 
+    @defer.inlineCallbacks
+    def test_blob_path_consistency(self):
+        """Test to make sure that paths used to store files is the same as the one used to get
+        already uploaded files.
+        """
+        assert_gcs_environ()
+        try:
+            import google.cloud.storage # noqa
+        except ModuleNotFoundError:
+            raise unittest.SkipTest("google-cloud-storage is not installed")
+        else:
+            with mock.patch('google.cloud.storage') as _:
+                with mock.patch('scrapy.pipelines.files.time') as _:
+                    uri = 'gs://my_bucket/my_prefix/'
+                    store = GCSFilesStore(uri)
+                    store.bucket = mock.Mock()
+                    path = 'full/my_data.txt'
+                    yield store.persist_file(path, mock.Mock(), info=None, meta=None, headers=None)
+                    yield store.stat_file(path, info=None)
+                    expected_blob_path = store.prefix + path
+                    store.bucket.blob.assert_called_with(expected_blob_path)
+                    store.bucket.get_blob.assert_called_with(expected_blob_path)
+
 
 class TestFTPFileStore(unittest.TestCase):
     @defer.inlineCallbacks
diff --git a/tox.ini b/tox.ini
index d13bb7b38..6951b6d16 100644
--- a/tox.ini
+++ b/tox.ini
@@ -126,13 +126,14 @@ setenv =
 deps =
     {[testenv]deps}
     boto
+    google-cloud-storage
+    # Twisted[http2] currently forces old mitmproxy because of h2 version
+    # restrictions in their deps, so we need to pin old markupsafe here too.
+    markupsafe < 2.1.0
     reppy
     robotexclusionrulesparser
     Pillow>=4.0.0
     Twisted[http2]>=17.9.0
-    # Twisted[http2] currently forces old mitmproxy because of h2 version restrictions in their deps,
-    # so we need to pin old markupsafe here too
-    markupsafe < 2.1.0
 
 [testenv:asyncio]
 commands =