From 5a55c4269d3389df4e486657caa58985f8e37e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Thu, 31 Jan 2019 17:20:29 -0200 Subject: [PATCH 01/56] Adding GCSFeedStorage --- scrapy/extensions/feedexport.py | 26 ++++++++++++++++++ tests/test_feedexport.py | 48 ++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 22ebf3b3f..00d5d8025 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -146,6 +146,32 @@ class S3FeedStorage(BlockingFeedStorage): key.close() + +class GCSFeedStorage(BlockingFeedStorage): + + project_id = None + bucket_name = None + blob_name = None + + def __init__(self, uri, project_id): + self.project_id = project_id + u = urlparse(uri) + self.bucket_name = u.hostname + self.blob_name = u.path[1:] # remove first "/" + + @classmethod + def from_crawler(cls, crawler, uri): + return cls(uri, crawler.settings['GCS_PROJECT_ID']) + + def _store_in_thread(self, file): + file.seek(0) + from google.cloud.storage import Client + client = Client(project=self.project_id) + bucket = client.get_bucket(self.bucket_name) + blob = bucket.blob(self.blob_name) + blob.upload_from_file(file) + + class FTPFeedStorage(BlockingFeedStorage): def __init__(self, uri): diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index e46c8c14e..f3c499b3f 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -21,7 +21,7 @@ from w3lib.url import path_to_file_uri import scrapy from scrapy.exporters import CsvItemExporter from scrapy.extensions.feedexport import ( - IFeedStorage, FileFeedStorage, FTPFeedStorage, + IFeedStorage, FileFeedStorage, FTPFeedStorage, GCSFeedStorage, S3FeedStorage, StdoutFeedStorage, BlockingFeedStorage) from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete, get_crawler @@ -187,6 +187,52 @@ class S3FeedStorageTest(unittest.TestCase): self.assertEqual(content, expected_content) +class GCSFeedStorageTest(unittest.TestCase): + + @mock.patch('scrapy.conf.settings', + new={'GCS_PROJECT_ID': 'conf_id' }, create=True) + def test_parse_settings(self): + try: + from google.cloud.storage import Client + except ImportError: + raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") + + settings = {'GCS_PROJECT_ID': '123' } + crawler = get_crawler(settings_dict=settings) + storage = GCSFeedStorage.from_crawler(crawler, 'gcs://mybucket/export.csv') + assert storage.project_id == '123' + assert storage.bucket_name == 'mybucket' + assert storage.blob_name == 'export.csv' + + @defer.inlineCallbacks + def test_store(self): + try: + from google.cloud.storage import Client, Bucket, Blob + except ImportError: + raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") + + uri = 'gcs://mybucket/export.csv' + project_id = 'myproject-123' + with mock.patch('google.cloud.storage.Client') as m: + client_mock = mock.create_autospec(Client) + m.return_value = client_mock + + bucket_mock = mock.create_autospec(Bucket) + client_mock.get_bucket.return_value = bucket_mock + + blob_mock = mock.create_autospec(Blob) + bucket_mock.blob.return_value = blob_mock + + f = mock.Mock() + storage = GCSFeedStorage(uri, project_id) + yield storage.store(f) + + f.seek.assert_called_once_with(0) + m.assert_called_once_with(project=project_id) + client_mock.get_bucket.assert_called_once_with('mybucket') + bucket_mock.blob.assert_called_once_with('export.csv') + blob_mock.upload_from_file.assert_called_once_with(f) + class StdoutFeedStorageTest(unittest.TestCase): @defer.inlineCallbacks From a4059851e7b6c8d712b2bc73dbff99be2d569d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Thu, 31 Jan 2019 18:29:15 -0200 Subject: [PATCH 02/56] Refactoring tests --- scrapy/utils/test.py | 16 ++++++++++++++++ tests/test_feedexport.py | 15 +++++---------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 4b935c51b..84eae97a7 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -7,6 +7,7 @@ import os from importlib import import_module from twisted.trial.unittest import SkipTest +from tests import mock from scrapy.exceptions import NotConfigured from scrapy.utils.boto import is_botocore @@ -91,3 +92,18 @@ def assert_samelines(testcase, text1, text2, msg=None): line endings between platforms """ testcase.assertEqual(text1.splitlines(), text2.splitlines(), msg) + +def mock_google_cloud_storage(): + """Creates autospec mocks for google-cloud-storage Client, Bucket and Blob + classes and set their proper return values. + """ + from google.cloud.storage import Client, Bucket, Blob + client_mock = mock.create_autospec(Client) + + bucket_mock = mock.create_autospec(Bucket) + client_mock.get_bucket.return_value = bucket_mock + + blob_mock = mock.create_autospec(Blob) + bucket_mock.blob.return_value = blob_mock + + return (client_mock, bucket_mock, blob_mock) \ No newline at end of file diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index f3c499b3f..6d23c68c3 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -24,7 +24,8 @@ from scrapy.extensions.feedexport import ( IFeedStorage, FileFeedStorage, FTPFeedStorage, GCSFeedStorage, S3FeedStorage, StdoutFeedStorage, BlockingFeedStorage) -from scrapy.utils.test import assert_aws_environ, get_s3_content_and_delete, get_crawler +from scrapy.utils.test import (assert_aws_environ, get_s3_content_and_delete, + get_crawler, mock_google_cloud_storage) from scrapy.utils.python import to_native_str @@ -207,22 +208,16 @@ class GCSFeedStorageTest(unittest.TestCase): @defer.inlineCallbacks def test_store(self): try: - from google.cloud.storage import Client, Bucket, Blob + from google.cloud.storage import Client except ImportError: raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") uri = 'gcs://mybucket/export.csv' project_id = 'myproject-123' - with mock.patch('google.cloud.storage.Client') as m: - client_mock = mock.create_autospec(Client) + (client_mock, bucket_mock, blob_mock) = mock_google_cloud_storage() + with mock.patch('google.cloud.storage.Client') as m: m.return_value = client_mock - bucket_mock = mock.create_autospec(Bucket) - client_mock.get_bucket.return_value = bucket_mock - - blob_mock = mock.create_autospec(Blob) - bucket_mock.blob.return_value = blob_mock - f = mock.Mock() storage = GCSFeedStorage(uri, project_id) yield storage.store(f) From 1bb6c4154c43d03d00147d810147c4f3c807505e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 8 Feb 2019 09:04:01 -0200 Subject: [PATCH 03/56] Turning into instance attributes --- scrapy/extensions/feedexport.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 00d5d8025..a81f44045 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -149,10 +149,6 @@ class S3FeedStorage(BlockingFeedStorage): class GCSFeedStorage(BlockingFeedStorage): - project_id = None - bucket_name = None - blob_name = None - def __init__(self, uri, project_id): self.project_id = project_id u = urlparse(uri) From fc6809b024dc25663432b6c0c5780021a827ea20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 8 Feb 2019 09:08:54 -0200 Subject: [PATCH 04/56] Add gcs schema to FEED_STORAGES_BASE --- scrapy/settings/default_settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 3734a0a58..1a12f35a3 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -145,6 +145,7 @@ FEED_STORAGES_BASE = { 'stdout': 'scrapy.extensions.feedexport.StdoutFeedStorage', 's3': 'scrapy.extensions.feedexport.S3FeedStorage', 'ftp': 'scrapy.extensions.feedexport.FTPFeedStorage', + 'gcs': 'scrapy.extensions.feedexport.GCSFeedStorage', } FEED_EXPORTERS = {} FEED_EXPORTERS_BASE = { From 4a53de165a53433a9a2a8cc4db34e5507c47fbd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 8 Feb 2019 09:09:56 -0200 Subject: [PATCH 05/56] Sorted schemas alphabetically --- scrapy/extensions/feedexport.py | 1 - scrapy/settings/default_settings.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index a81f44045..8347b42ca 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -146,7 +146,6 @@ class S3FeedStorage(BlockingFeedStorage): key.close() - class GCSFeedStorage(BlockingFeedStorage): def __init__(self, uri, project_id): diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 1a12f35a3..8769c01ba 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -142,10 +142,10 @@ FEED_STORAGES = {} FEED_STORAGES_BASE = { '': 'scrapy.extensions.feedexport.FileFeedStorage', 'file': 'scrapy.extensions.feedexport.FileFeedStorage', - 'stdout': 'scrapy.extensions.feedexport.StdoutFeedStorage', - 's3': 'scrapy.extensions.feedexport.S3FeedStorage', 'ftp': 'scrapy.extensions.feedexport.FTPFeedStorage', 'gcs': 'scrapy.extensions.feedexport.GCSFeedStorage', + 's3': 'scrapy.extensions.feedexport.S3FeedStorage', + 'stdout': 'scrapy.extensions.feedexport.StdoutFeedStorage', } FEED_EXPORTERS = {} FEED_EXPORTERS_BASE = { From 2bbbd02bda368c2f052f3aa38f99498a632328bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 8 Feb 2019 09:45:10 -0200 Subject: [PATCH 06/56] Adding an option to set ACL while uploading the blob to GCS --- scrapy/extensions/feedexport.py | 11 ++++++++--- scrapy/settings/default_settings.py | 2 ++ tests/test_feedexport.py | 10 ++++++---- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 8347b42ca..fbbf9bb97 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -148,15 +148,20 @@ class S3FeedStorage(BlockingFeedStorage): class GCSFeedStorage(BlockingFeedStorage): - def __init__(self, uri, project_id): + def __init__(self, uri, project_id, acl): self.project_id = project_id + self.acl = acl u = urlparse(uri) self.bucket_name = u.hostname self.blob_name = u.path[1:] # remove first "/" @classmethod def from_crawler(cls, crawler, uri): - return cls(uri, crawler.settings['GCS_PROJECT_ID']) + return cls( + uri, + crawler.settings['GCS_PROJECT_ID'], + crawler.settings['FEED_STORAGE_GCS_ACL'] + ) def _store_in_thread(self, file): file.seek(0) @@ -164,7 +169,7 @@ class GCSFeedStorage(BlockingFeedStorage): client = Client(project=self.project_id) bucket = client.get_bucket(self.bucket_name) blob = bucket.blob(self.blob_name) - blob.upload_from_file(file) + blob.upload_from_file(file, predefined_acl=self.acl) class FTPFeedStorage(BlockingFeedStorage): diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 8769c01ba..5d2862980 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -159,6 +159,8 @@ FEED_EXPORTERS_BASE = { } FEED_EXPORT_INDENT = 0 +FEED_STORAGE_GCS_ACL = None + FILES_STORE_S3_ACL = 'private' FILES_STORE_GCS_ACL = '' diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 6d23c68c3..5cbca6d28 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -191,17 +191,18 @@ class S3FeedStorageTest(unittest.TestCase): class GCSFeedStorageTest(unittest.TestCase): @mock.patch('scrapy.conf.settings', - new={'GCS_PROJECT_ID': 'conf_id' }, create=True) + new={'GCS_PROJECT_ID': 'conf_id', 'FEED_STORAGE_GCS_ACL': None }, create=True) def test_parse_settings(self): try: from google.cloud.storage import Client except ImportError: raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") - settings = {'GCS_PROJECT_ID': '123' } + settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': 'publicRead' } crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, 'gcs://mybucket/export.csv') assert storage.project_id == '123' + assert storage.acl == 'publicRead' assert storage.bucket_name == 'mybucket' assert storage.blob_name == 'export.csv' @@ -214,19 +215,20 @@ class GCSFeedStorageTest(unittest.TestCase): uri = 'gcs://mybucket/export.csv' project_id = 'myproject-123' + acl = 'publicRead' (client_mock, bucket_mock, blob_mock) = mock_google_cloud_storage() with mock.patch('google.cloud.storage.Client') as m: m.return_value = client_mock f = mock.Mock() - storage = GCSFeedStorage(uri, project_id) + storage = GCSFeedStorage(uri, project_id, acl) yield storage.store(f) f.seek.assert_called_once_with(0) m.assert_called_once_with(project=project_id) client_mock.get_bucket.assert_called_once_with('mybucket') bucket_mock.blob.assert_called_once_with('export.csv') - blob_mock.upload_from_file.assert_called_once_with(f) + blob_mock.upload_from_file.assert_called_once_with(f, predefined_acl=acl) class StdoutFeedStorageTest(unittest.TestCase): From cb5f800b0f7029d2bf2e09e9bc1391078c133a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 8 Feb 2019 11:26:33 -0200 Subject: [PATCH 07/56] Adding documentation about Google Cloud Storage Feed Export --- docs/topics/feed-exports.rst | 22 ++++++++++++++++++++++ docs/topics/settings.rst | 18 ++++++++++++++++++ scrapy/settings/default_settings.py | 2 ++ 3 files changed, 42 insertions(+) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index b64dbfbfd..efb63b0ba 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -185,6 +185,27 @@ passed through the following settings: * :setting:`AWS_ACCESS_KEY_ID` * :setting:`AWS_SECRET_ACCESS_KEY` +.. _topics-feed-storage-gcs: + +Google Cloud Storage (GCS) +-------------------------- + +The feeds are stored on `Google Cloud Storage`_. + + * URI scheme: ``gcs`` + * Example URIs: + + * ``gcs://mybucket/path/to/export.csv`` + + * Required external libraries: `google-cloud-storage `_. + +For more information about authentication, please refer to `Google Cloud documentation `_. + +You can set a *Project ID* and *Access Control List (ACL)* through the following settings: + + * :setting:`FEED_STORAGE_GCS_ACL` + * :setting:`GCS_PROJECT_ID` + .. _topics-feed-storage-stdout: Standard output @@ -366,3 +387,4 @@ format in :setting:`FEED_EXPORTERS`. E.g., to disable the built-in CSV exporter .. _Amazon S3: https://aws.amazon.com/s3/ .. _boto: https://github.com/boto/boto .. _botocore: https://github.com/boto/botocore +.. _Google Cloud Storage: https://cloud.google.com/storage/ \ No newline at end of file diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 0ac26a9bd..90ae8fd93 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -749,6 +749,14 @@ The Feed Temp dir allows you to set a custom folder to save crawler temporary files before uploading with :ref:`FTP feed storage ` and :ref:`Amazon S3 `. +.. setting:: FEED_STORAGE_GCS_ACL + +FEED_STORAGE_GCS_ACL +-------------------- + +The Access Control List (ACL) used when storing items to :ref:`Google Cloud Storage `. +For more information on how to set this value, please refer to `Google Cloud documentation `_. + .. setting:: FTP_PASSIVE_MODE FTP_PASSIVE_MODE @@ -786,6 +794,15 @@ Default: ``"anonymous"`` The username to use for FTP connections when there is no ``"ftp_user"`` in ``Request`` meta. +.. setting:: GCS_PROJECT_ID + +GCS_PROJECT_ID +----------------- + +Default: ``None`` + +The Project ID that will be used when storing data on `Google Cloud Storage`_. + .. setting:: ITEM_PIPELINES ITEM_PIPELINES @@ -1371,3 +1388,4 @@ case to see how to enable and use them. .. _Amazon web services: https://aws.amazon.com/ .. _breadth-first order: https://en.wikipedia.org/wiki/Breadth-first_search .. _depth-first order: https://en.wikipedia.org/wiki/Depth-first_search +.. _Google Cloud Storage: https://cloud.google.com/storage/ \ No newline at end of file diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 5d2862980..c17e94a64 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -168,6 +168,8 @@ FTP_USER = 'anonymous' FTP_PASSWORD = 'guest' FTP_PASSIVE_MODE = True +GCS_PROJECT_ID = None + HTTPCACHE_ENABLED = False HTTPCACHE_DIR = 'httpcache' HTTPCACHE_IGNORE_MISSING = False From 0bb3d8ca93cf68e0ef231ee734f8a1a3c4075a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Wed, 27 Feb 2019 18:41:01 -0300 Subject: [PATCH 08/56] Updating Google Cloud Storage scheme to gs instead of gcs --- docs/topics/feed-exports.rst | 4 ++-- scrapy/settings/default_settings.py | 2 +- tests/test_feedexport.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index efb63b0ba..0957a5997 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -192,10 +192,10 @@ Google Cloud Storage (GCS) The feeds are stored on `Google Cloud Storage`_. - * URI scheme: ``gcs`` + * URI scheme: ``gs`` * Example URIs: - * ``gcs://mybucket/path/to/export.csv`` + * ``gs://mybucket/path/to/export.csv`` * Required external libraries: `google-cloud-storage `_. diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index c17e94a64..50fcd1d0a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -143,7 +143,7 @@ FEED_STORAGES_BASE = { '': 'scrapy.extensions.feedexport.FileFeedStorage', 'file': 'scrapy.extensions.feedexport.FileFeedStorage', 'ftp': 'scrapy.extensions.feedexport.FTPFeedStorage', - 'gcs': 'scrapy.extensions.feedexport.GCSFeedStorage', + 'gs': 'scrapy.extensions.feedexport.GCSFeedStorage', 's3': 'scrapy.extensions.feedexport.S3FeedStorage', 'stdout': 'scrapy.extensions.feedexport.StdoutFeedStorage', } diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 5cbca6d28..41df7d7af 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -200,7 +200,7 @@ class GCSFeedStorageTest(unittest.TestCase): settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': 'publicRead' } crawler = get_crawler(settings_dict=settings) - storage = GCSFeedStorage.from_crawler(crawler, 'gcs://mybucket/export.csv') + storage = GCSFeedStorage.from_crawler(crawler, 'gs://mybucket/export.csv') assert storage.project_id == '123' assert storage.acl == 'publicRead' assert storage.bucket_name == 'mybucket' From 2cb4dc32052c306568206f0997de4b4e53069efd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Fri, 22 Mar 2019 09:50:11 -0300 Subject: [PATCH 09/56] Mentioning to use JSON API for ACLs --- docs/topics/settings.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 90ae8fd93..fcdf31cac 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -755,7 +755,7 @@ FEED_STORAGE_GCS_ACL -------------------- The Access Control List (ACL) used when storing items to :ref:`Google Cloud Storage `. -For more information on how to set this value, please refer to `Google Cloud documentation `_. +For more information on how to set this value, please refer to the column *JSON API* in `Google Cloud documentation `_. .. setting:: FTP_PASSIVE_MODE @@ -1388,4 +1388,4 @@ case to see how to enable and use them. .. _Amazon web services: https://aws.amazon.com/ .. _breadth-first order: https://en.wikipedia.org/wiki/Breadth-first_search .. _depth-first order: https://en.wikipedia.org/wiki/Depth-first_search -.. _Google Cloud Storage: https://cloud.google.com/storage/ \ No newline at end of file +.. _Google Cloud Storage: https://cloud.google.com/storage/ From 110bc92e6b9c9c3ce1775a9ca1487df42a80d219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= Date: Thu, 29 Aug 2019 11:10:00 -0300 Subject: [PATCH 10/56] Fix default value of FEED_STORAGE_GCS_ACL --- scrapy/extensions/feedexport.py | 2 +- scrapy/settings/default_settings.py | 2 +- tests/test_feedexport.py | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index fbbf9bb97..1e982c684 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -160,7 +160,7 @@ class GCSFeedStorage(BlockingFeedStorage): return cls( uri, crawler.settings['GCS_PROJECT_ID'], - crawler.settings['FEED_STORAGE_GCS_ACL'] + crawler.settings['FEED_STORAGE_GCS_ACL'] or None ) def _store_in_thread(self, file): diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 50fcd1d0a..45257a61c 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -159,7 +159,7 @@ FEED_EXPORTERS_BASE = { } FEED_EXPORT_INDENT = 0 -FEED_STORAGE_GCS_ACL = None +FEED_STORAGE_GCS_ACL = '' FILES_STORE_S3_ACL = 'private' FILES_STORE_GCS_ACL = '' diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 41df7d7af..69f144d07 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -206,6 +206,24 @@ class GCSFeedStorageTest(unittest.TestCase): assert storage.bucket_name == 'mybucket' assert storage.blob_name == 'export.csv' + @mock.patch('scrapy.conf.settings', + new={'GCS_PROJECT_ID': 'conf_id', 'FEED_STORAGE_GCS_ACL': '' }, create=True) + def test_parse_empty_acl(self): + try: + from google.cloud.storage import Client + except ImportError: + raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") + + settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': '' } + crawler = get_crawler(settings_dict=settings) + storage = GCSFeedStorage.from_crawler(crawler, 'gs://mybucket/export.csv') + assert storage.acl is None + + settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': None } + crawler = get_crawler(settings_dict=settings) + storage = GCSFeedStorage.from_crawler(crawler, 'gs://mybucket/export.csv') + assert storage.acl is None + @defer.inlineCallbacks def test_store(self): try: From b9a58798eed39bf543b9682d9ce43b13378a5074 Mon Sep 17 00:00:00 2001 From: nyov Date: Sat, 24 May 2014 20:24:01 +0000 Subject: [PATCH 11/56] change Scraper API to call internal `_parse` method A Spider class using internal pre-processing can have first dibs at this and then call a public `parse` method for subclass hooking. --- scrapy/core/scraper.py | 2 +- scrapy/spiders/__init__.py | 3 +++ scrapy/spiders/crawl.py | 2 +- scrapy/spiders/feed.py | 4 ++-- tests/test_spider.py | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 99114d3bb..ad6649134 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -145,7 +145,7 @@ class Scraper(object): def call_spider(self, result, request, spider): result.request = request dfd = defer_result(result) - callback = request.callback or spider.parse + callback = request.callback or spider._parse warn_on_generator_with_return_value(spider, callback) warn_on_generator_with_return_value(spider, request.errback) dfd.addCallbacks(callback=callback, diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 9429f6cb2..1011eb870 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -80,6 +80,9 @@ class Spider(object_ref): """ This method is deprecated. """ return Request(url, dont_filter=True) + def _parse(self, response): + return self.parse(response) + def parse(self, response): raise NotImplementedError('{}.parse callback is not defined'.format(self.__class__.__name__)) diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index a2c364c0e..e28d17dcd 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -74,7 +74,7 @@ class CrawlSpider(Spider): super(CrawlSpider, self).__init__(*a, **kw) self._compile_rules() - def parse(self, response): + def _parse(self, response): return self._parse_response(response, self.parse_start_url, cb_kwargs={}, follow=True) def parse_start_url(self, response): diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index c566f0236..11bd17db4 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -61,7 +61,7 @@ class XMLFeedSpider(Spider): for result_item in self.process_results(response, ret): yield result_item - def parse(self, response): + def _parse(self, response): if not hasattr(self, 'parse_node'): raise NotConfigured('You must define parse_node method in order to scrape this XML feed') @@ -128,7 +128,7 @@ class CSVFeedSpider(Spider): for result_item in self.process_results(response, ret): yield result_item - def parse(self, response): + def _parse(self, response): if not hasattr(self, 'parse_row'): raise NotConfigured('You must define parse_row method in order to scrape this CSV feed') response = self.adapt_response(response) diff --git a/tests/test_spider.py b/tests/test_spider.py index 317a27076..6fbec7e58 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -142,7 +142,7 @@ class XMLFeedSpiderTest(SpiderTest): for iterator in ('iternodes', 'xml'): spider = _XMLSpider('example', iterator=iterator) - output = list(spider.parse(response)) + output = list(spider._parse(response)) self.assertEqual(len(output), 2, iterator) self.assertEqual(output, [ {'loc': [u'http://www.example.com/Special-Offers.html'], From 5982e3477c732f4dff5accea6eab486e4ab52c3e Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Mon, 23 Dec 2019 14:12:21 -0300 Subject: [PATCH 12/56] Take keyword arguments in base parsing methods --- docs/topics/spiders.rst | 12 +++++++----- scrapy/spiders/__init__.py | 6 +++--- scrapy/spiders/crawl.py | 11 ++++++++--- scrapy/spiders/feed.py | 4 ++-- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index b0fb14e24..dd763b607 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -362,12 +362,14 @@ CrawlSpider This spider also exposes an overrideable method: - .. method:: parse_start_url(response) + .. method:: parse_start_url(response, **kwargs) - This method is called for the start_urls responses. It allows to parse - the initial responses and must return either an - :class:`~scrapy.item.Item` object, a :class:`~scrapy.http.Request` - object, or an iterable containing any of them. + This method is called for each response produced for the URLs in + the spider's ``start_urls`` attribute. It allows to parse + the initial responses and must return either an item + (:class:`scrapy.item.Item` or :class:`dict`), + a :class:`~scrapy.http.Request`, + or an iterable containing any of them. Crawling rules ~~~~~~~~~~~~~~ diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 1011eb870..3e19f1e23 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -80,10 +80,10 @@ class Spider(object_ref): """ This method is deprecated. """ return Request(url, dont_filter=True) - def _parse(self, response): - return self.parse(response) + def _parse(self, response, **kwargs): + return self.parse(response, **kwargs) - def parse(self, response): + def parse(self, response, **kwargs): raise NotImplementedError('{}.parse callback is not defined'.format(self.__class__.__name__)) @classmethod diff --git a/scrapy/spiders/crawl.py b/scrapy/spiders/crawl.py index e28d17dcd..4ec0de78c 100644 --- a/scrapy/spiders/crawl.py +++ b/scrapy/spiders/crawl.py @@ -74,10 +74,15 @@ class CrawlSpider(Spider): super(CrawlSpider, self).__init__(*a, **kw) self._compile_rules() - def _parse(self, response): - return self._parse_response(response, self.parse_start_url, cb_kwargs={}, follow=True) + def _parse(self, response, **kwargs): + return self._parse_response( + response=response, + callback=self.parse_start_url, + cb_kwargs=kwargs, + follow=True, + ) - def parse_start_url(self, response): + def parse_start_url(self, response, **kwargs): return [] def process_results(self, response, results): diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 11bd17db4..4fa6009a5 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -61,7 +61,7 @@ class XMLFeedSpider(Spider): for result_item in self.process_results(response, ret): yield result_item - def _parse(self, response): + def _parse(self, response, **kwargs): if not hasattr(self, 'parse_node'): raise NotConfigured('You must define parse_node method in order to scrape this XML feed') @@ -128,7 +128,7 @@ class CSVFeedSpider(Spider): for result_item in self.process_results(response, ret): yield result_item - def _parse(self, response): + def _parse(self, response, **kwargs): if not hasattr(self, 'parse_row'): raise NotConfigured('You must define parse_row method in order to scrape this CSV feed') response = self.adapt_response(response) From 8d4948f6ca44a76ee7714c8b4b1c46ef73a8845e Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 26 Dec 2019 14:38:11 -0300 Subject: [PATCH 13/56] [test] Override CrawlSpider.parse --- tests/spiders.py | 35 +++++++++++++++++++++++++++++------ tests/test_crawl.py | 31 +++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/tests/spiders.py b/tests/spiders.py index 39c8da0b6..dcc475ca7 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -186,13 +186,39 @@ class DuplicateStartRequestsSpider(MockServerSpider): self.visited += 1 -class CrawlSpiderWithErrback(MockServerSpider, CrawlSpider): - name = 'crawl_spider_with_errback' +class CrawlSpiderWithParseMethod(MockServerSpider, CrawlSpider): + """ + A CrawlSpider which overrides the 'parse' method + """ + name = 'crawl_spider_with_parse_method' custom_settings = { 'RETRY_HTTP_CODES': [], # no need to retry } rules = ( - Rule(LinkExtractor(), callback='callback', errback='errback', follow=True), + Rule(LinkExtractor(), callback='parse', follow=True), + ) + + def start_requests(self): + test_body = b""" + + Page title<title></head> + <body> + <p><a href="/status?n=200">Item 200</a></p> <!-- callback --> + <p><a href="/status?n=201">Item 201</a></p> <!-- callback --> + </body> + </html> + """ + url = self.mockserver.url("/alpayload") + yield Request(url, method="POST", body=test_body) + + def parse(self, response): + self.logger.info('[parse] status %i', response.status) + + +class CrawlSpiderWithErrback(CrawlSpiderWithParseMethod): + name = 'crawl_spider_with_errback' + rules = ( + Rule(LinkExtractor(), callback='parse', errback='errback', follow=True), ) def start_requests(self): @@ -211,8 +237,5 @@ class CrawlSpiderWithErrback(MockServerSpider, CrawlSpider): url = self.mockserver.url("/alpayload") yield Request(url, method="POST", body=test_body) - def callback(self, response): - self.logger.info('[callback] status %i', response.status) - def errback(self, failure): self.logger.info('[errback] status %i', failure.value.response.status) diff --git a/tests/test_crawl.py b/tests/test_crawl.py index f433fcea6..4299e4bbb 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -9,8 +9,9 @@ from scrapy.crawler import CrawlerRunner from scrapy.http import Request from scrapy.utils.python import to_unicode from tests.mockserver import MockServer -from tests.spiders import (FollowAllSpider, DelaySpider, SimpleSpider, BrokenStartRequestsSpider, - SingleRequestSpider, DuplicateStartRequestsSpider, CrawlSpiderWithErrback) +from tests.spiders import (BrokenStartRequestsSpider, CrawlSpiderWithErrback, + CrawlSpiderWithParseMethod, DelaySpider, SimpleSpider, + DuplicateStartRequestsSpider, FollowAllSpider, SingleRequestSpider) class CrawlTestCase(TestCase): @@ -297,6 +298,27 @@ with multiples lines self._assert_retried(log) self.assertIn("Got response 200", str(log)) + +class CrawlSpiderTestCase(TestCase): + + def setUp(self): + self.mockserver = MockServer() + self.mockserver.__enter__() + self.runner = CrawlerRunner() + + def tearDown(self): + self.mockserver.__exit__(None, None, None) + + @defer.inlineCallbacks + def test_crawlspider_with_parse(self): + self.runner.crawl(CrawlSpiderWithParseMethod, mockserver=self.mockserver) + + with LogCapture() as log: + yield self.runner.join() + + self.assertIn("[parse] status 200", str(log)) + self.assertIn("[parse] status 201", str(log)) + @defer.inlineCallbacks def test_crawlspider_with_errback(self): self.runner.crawl(CrawlSpiderWithErrback, mockserver=self.mockserver) @@ -304,7 +326,8 @@ with multiples lines with LogCapture() as log: yield self.runner.join() - self.assertIn("[callback] status 200", str(log)) - self.assertIn("[callback] status 201", str(log)) + self.assertIn("[parse] status 200", str(log)) + self.assertIn("[parse] status 201", str(log)) self.assertIn("[errback] status 404", str(log)) self.assertIn("[errback] status 500", str(log)) + self.assertIn("[errback] status 501", str(log)) From c54df8253a67bb6863a25bf7d667096adc73a040 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Thu, 26 Dec 2019 15:12:19 -0300 Subject: [PATCH 14/56] [test] Handle keyword args in CrawlSpider.parse --- tests/spiders.py | 5 +++-- tests/test_crawl.py | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/spiders.py b/tests/spiders.py index dcc475ca7..c042eb7fe 100644 --- a/tests/spiders.py +++ b/tests/spiders.py @@ -211,8 +211,9 @@ class CrawlSpiderWithParseMethod(MockServerSpider, CrawlSpider): url = self.mockserver.url("/alpayload") yield Request(url, method="POST", body=test_body) - def parse(self, response): - self.logger.info('[parse] status %i', response.status) + def parse(self, response, foo=None): + self.logger.info('[parse] status %i (foo: %s)', response.status, foo) + yield Request(self.mockserver.url("/status?n=202"), self.parse, cb_kwargs={"foo": "bar"}) class CrawlSpiderWithErrback(CrawlSpiderWithParseMethod): diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 4299e4bbb..6247ced35 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -316,8 +316,9 @@ class CrawlSpiderTestCase(TestCase): with LogCapture() as log: yield self.runner.join() - self.assertIn("[parse] status 200", str(log)) - self.assertIn("[parse] status 201", str(log)) + self.assertIn("[parse] status 200 (foo: None)", str(log)) + self.assertIn("[parse] status 201 (foo: None)", str(log)) + self.assertIn("[parse] status 202 (foo: bar)", str(log)) @defer.inlineCallbacks def test_crawlspider_with_errback(self): @@ -326,8 +327,9 @@ class CrawlSpiderTestCase(TestCase): with LogCapture() as log: yield self.runner.join() - self.assertIn("[parse] status 200", str(log)) - self.assertIn("[parse] status 201", str(log)) + self.assertIn("[parse] status 200 (foo: None)", str(log)) + self.assertIn("[parse] status 201 (foo: None)", str(log)) + self.assertIn("[parse] status 202 (foo: bar)", str(log)) self.assertIn("[errback] status 404", str(log)) self.assertIn("[errback] status 500", str(log)) self.assertIn("[errback] status 501", str(log)) From 8a1dc26d4662ec0c81d22c13cffa44255a7c29ca Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Thu, 26 Dec 2019 15:14:47 -0300 Subject: [PATCH 15/56] [doc] Note about the 'parse' method for CrawlSpider/XMLFeedSpider --- docs/topics/spiders.rst | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index dd763b607..406f50fb3 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -392,11 +392,6 @@ Crawling rules object will contain the text of the link that produced the :class:`~scrapy.http.Request` in its ``meta`` dictionary (under the ``link_text`` key) - .. warning:: When writing crawl spider rules, avoid using ``parse`` as - callback, since the :class:`CrawlSpider` uses the ``parse`` method - itself to implement its logic. So if you override the ``parse`` method, - the crawl spider will no longer work. - ``cb_kwargs`` is a dict containing the keyword arguments to be passed to the callback function. @@ -422,6 +417,12 @@ Crawling rules It receives a :class:`Twisted Failure <twisted.python.failure.Failure>` instance as first parameter. + +.. warning:: Because of its internal implementation, you must explicitly set + callbacks for new requests when writing :class:`CrawlSpider`-based spiders; + unexpected behaviour can occur otherwise. + + CrawlSpider example ~~~~~~~~~~~~~~~~~~~ @@ -452,6 +453,11 @@ Let's now take a look at an example CrawlSpider with rules:: item['name'] = response.xpath('//td[@id="item_name"]/text()').get() item['description'] = response.xpath('//td[@id="item_description"]/text()').get() item['link_text'] = response.meta['link_text'] + url = response.xpath('//td[@id="additional_data"]/@href').get() + return response.follow(url, self.parse_additional_page, cb_kwargs=dict(item=item)) + + def parse_additional_page(self, response, item): + item['additional_data'] = response.xpath('//p[@id="additional_data"]/text()').get() return item @@ -545,6 +551,11 @@ XMLFeedSpider those results. It must return a list of results (Items or Requests). +.. warning:: Because of its internal implementation, you must explicitly set + callbacks for new requests when writing :class:`XMLFeedSpider`-based spiders; + unexpected behaviour can occur otherwise. + + XMLFeedSpider example ~~~~~~~~~~~~~~~~~~~~~ From 7f2d3051feb0bc8f868a3eea8310e2fc8c461287 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Thu, 6 Feb 2020 18:19:40 -0300 Subject: [PATCH 16/56] Fix Flake8 issue --- tests/test_crawl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_crawl.py b/tests/test_crawl.py index f1d502ff7..225fe7a0e 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -14,8 +14,8 @@ from tests.mockserver import MockServer from tests.spiders import ( AsyncDefAsyncioReturnSpider, AsyncDefAsyncioSpider, - AsyncDefSpider, - BrokenStartRequestsSpider, + AsyncDefSpider, + BrokenStartRequestsSpider, CrawlSpiderWithErrback, CrawlSpiderWithParseMethod, DelaySpider, From 7025c18b159f1ce14b5732dc7d262efbdc72cf5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Mon, 10 Feb 2020 19:43:23 +0100 Subject: [PATCH 17/56] Clear line of spaces --- docs/topics/feed-exports.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 9f837221e..8dc08710f 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -184,7 +184,7 @@ passed through the following settings: * :setting:`AWS_ACCESS_KEY_ID` * :setting:`AWS_SECRET_ACCESS_KEY` - + You can also define a custom ACL for exported feeds using this setting: * :setting:`FEED_STORAGE_S3_ACL` From 73e88d036c72aa628af50371aace4cfce7e286e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Wed, 12 Feb 2020 17:17:38 +0100 Subject: [PATCH 18/56] Import mock from unittest --- scrapy/utils/test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 4ff6d73a5..00002c303 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -2,14 +2,13 @@ This module contains some assorted functions used in tests """ -from __future__ import absolute_import -from posixpath import split import asyncio import os +from posixpath import split +from unittest import mock from importlib import import_module from twisted.trial.unittest import SkipTest -from tests import mock from scrapy.exceptions import NotConfigured from scrapy.utils.boto import is_botocore From e1be078eaa17a8df72716932fde07e225f79745f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Wed, 12 Feb 2020 17:38:06 +0100 Subject: [PATCH 19/56] Fix Flake8-reported issues --- scrapy/utils/test.py | 2 ++ tests/test_feedexport.py | 27 ++++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index 00002c303..7442a2f33 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -120,12 +120,14 @@ def assert_samelines(testcase, text1, text2, msg=None): """ testcase.assertEqual(text1.splitlines(), text2.splitlines(), msg) + def get_from_asyncio_queue(value): q = asyncio.Queue() getter = q.get() q.put_nowait(value) return getter + def mock_google_cloud_storage(): """Creates autospec mocks for google-cloud-storage Client, Bucket and Blob classes and set their proper return values. diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index d97b199fe..2b299503b 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -25,9 +25,13 @@ from scrapy.extensions.feedexport import ( IFeedStorage, FileFeedStorage, FTPFeedStorage, GCSFeedStorage, S3FeedStorage, StdoutFeedStorage, BlockingFeedStorage) -from scrapy.utils.test import (assert_aws_environ, get_s3_content_and_delete, - get_crawler, mock_google_cloud_storage) from scrapy.utils.python import to_unicode +from scrapy.utils.test import ( + assert_aws_environ, + get_s3_content_and_delete, + get_crawler, + mock_google_cloud_storage, +) class FileFeedStorageTest(unittest.TestCase): @@ -362,15 +366,13 @@ class S3FeedStorageTest(unittest.TestCase): class GCSFeedStorageTest(unittest.TestCase): - @mock.patch('scrapy.conf.settings', - new={'GCS_PROJECT_ID': 'conf_id', 'FEED_STORAGE_GCS_ACL': None }, create=True) def test_parse_settings(self): try: - from google.cloud.storage import Client + from google.cloud.storage import Client # noqa except ImportError: raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") - settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': 'publicRead' } + settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': 'publicRead'} crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, 'gs://mybucket/export.csv') assert storage.project_id == '123' @@ -378,20 +380,18 @@ class GCSFeedStorageTest(unittest.TestCase): assert storage.bucket_name == 'mybucket' assert storage.blob_name == 'export.csv' - @mock.patch('scrapy.conf.settings', - new={'GCS_PROJECT_ID': 'conf_id', 'FEED_STORAGE_GCS_ACL': '' }, create=True) def test_parse_empty_acl(self): try: - from google.cloud.storage import Client + from google.cloud.storage import Client # noqa except ImportError: raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") - settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': '' } + settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': ''} crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, 'gs://mybucket/export.csv') assert storage.acl is None - settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': None } + settings = {'GCS_PROJECT_ID': '123', 'FEED_STORAGE_GCS_ACL': None} crawler = get_crawler(settings_dict=settings) storage = GCSFeedStorage.from_crawler(crawler, 'gs://mybucket/export.csv') assert storage.acl is None @@ -399,7 +399,7 @@ class GCSFeedStorageTest(unittest.TestCase): @defer.inlineCallbacks def test_store(self): try: - from google.cloud.storage import Client + from google.cloud.storage import Client # noqa except ImportError: raise unittest.SkipTest("GCSFeedStorage requires google-cloud-storage") @@ -407,7 +407,7 @@ class GCSFeedStorageTest(unittest.TestCase): project_id = 'myproject-123' acl = 'publicRead' (client_mock, bucket_mock, blob_mock) = mock_google_cloud_storage() - with mock.patch('google.cloud.storage.Client') as m: + with mock.patch('google.cloud.storage.Client') as m: m.return_value = client_mock f = mock.Mock() @@ -420,6 +420,7 @@ class GCSFeedStorageTest(unittest.TestCase): bucket_mock.blob.assert_called_once_with('export.csv') blob_mock.upload_from_file.assert_called_once_with(f, predefined_acl=acl) + class StdoutFeedStorageTest(unittest.TestCase): @defer.inlineCallbacks From a175b6efc319ecbfd03b19705c25bd8bd44ca339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= <dangra@gmail.com> Date: Fri, 27 Mar 2020 02:10:10 -0300 Subject: [PATCH 20/56] Set up CI with Azure Pipelines [skip ci] --- azure-pipelines.yml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 azure-pipelines.yml diff --git a/azure-pipelines.yml b/azure-pipelines.yml new file mode 100644 index 000000000..9bc324c32 --- /dev/null +++ b/azure-pipelines.yml @@ -0,0 +1,32 @@ +# Python package +# Create and test a Python package on multiple Python versions. +# Add steps that analyze code, save the dist with the build record, publish to a PyPI-compatible index, and more: +# https://docs.microsoft.com/azure/devops/pipelines/languages/python + + +pool: + vmImage: 'windows-2019' +strategy: + matrix: + Python35: + python.version: '3.5' + Python36: + python.version: '3.6' + Python37: + python.version: '3.7' + +steps: +- task: UsePythonVersion@0 + inputs: + versionSpec: '$(python.version)' + displayName: 'Use Python $(python.version)' + +- script: | + python -m pip install --upgrade pip + pip install -r requirements.txt + displayName: 'Install dependencies' + +- script: | + pip install pytest pytest-azurepipelines + pytest + displayName: 'pytest' From 02206e5ffe74fe8107c272ad35920928544323b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= <dangra@gmail.com> Date: Fri, 27 Mar 2020 02:20:39 -0300 Subject: [PATCH 21/56] Run tox --- azure-pipelines.yml | 9 ++++++--- tests/requirements-py3.txt | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 9bc324c32..489cfe53b 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -10,10 +10,13 @@ strategy: matrix: Python35: python.version: '3.5' + TOXENV: py35 Python36: python.version: '3.6' + TOXENV: py36 Python37: python.version: '3.7' + TOXENV: py37 steps: - task: UsePythonVersion@0 @@ -27,6 +30,6 @@ steps: displayName: 'Install dependencies' - script: | - pip install pytest pytest-azurepipelines - pytest - displayName: 'pytest' + pip install -U tox twine wheel codecov + tox + displayName: 'Run test suite' diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index d207c5fb0..8896f4614 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -6,6 +6,7 @@ pytest < 5.4 pytest-cov pytest-twisted >= 1.11 pytest-xdist +pytest-azurepipelines sybil testfixtures From 0699e6bb1600ff943e131a3b7a299aa038145198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= <dangra@gmail.com> Date: Fri, 27 Mar 2020 02:22:05 -0300 Subject: [PATCH 22/56] no need to install requirements.txt --- azure-pipelines.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 489cfe53b..ffc4d549b 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -24,11 +24,6 @@ steps: versionSpec: '$(python.version)' displayName: 'Use Python $(python.version)' -- script: | - python -m pip install --upgrade pip - pip install -r requirements.txt - displayName: 'Install dependencies' - - script: | pip install -U tox twine wheel codecov tox From 3fb0027138ab44b16be6e11677626d69b8c90c95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Sat, 28 Mar 2020 17:36:50 +0100 Subject: [PATCH 23/56] =?UTF-8?q?Require=20sybil=20=E2=89=A5=201.3.0=20for?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/requirements-py3.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index d207c5fb0..e7c86e0e9 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -6,7 +6,7 @@ pytest < 5.4 pytest-cov pytest-twisted >= 1.11 pytest-xdist -sybil +sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures # optional for shell wrapper tests From 2f510fd47d8d10217f7b18b531205dd8c252eaef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Wed, 15 Apr 2020 21:10:05 +0200 Subject: [PATCH 24/56] Fix ShellTest.test_local_file on Windows --- scrapy/utils/url.py | 13 +++++++------ tests/test_command_shell.py | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index c9abb12d5..c29ed4461 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -85,11 +85,9 @@ def add_http_if_no_scheme(url): def guess_scheme(url): - """Add an URL scheme if missing: file:// for filepath-like input or http:// otherwise.""" - parts = urlparse(url) - if parts.scheme: - return url - # Note: this does not match Windows filepath + """Add an URL scheme if missing: file:// for filepath-like input or + http:// otherwise.""" + # POSIX path if re.match(r'''^ # start with... ( \. # ...a single dot, @@ -99,7 +97,10 @@ def guess_scheme(url): )? # optional match of ".", ".." or ".blabla" / # at least one "/" for a file path, . # and something after the "/" - ''', parts.path, flags=re.VERBOSE): + ''', url, flags=re.VERBOSE): + return any_to_uri(url) + # Windows drive-letter path + elif re.match(r'''^[a-z]:\\''', url, flags=re.IGNORECASE): return any_to_uri(url) else: return add_http_if_no_scheme(url) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index d664b6ade..acf8e9f71 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -94,7 +94,7 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @defer.inlineCallbacks def test_local_file(self): - filepath = join(tests_datadir, 'test_site/index.html') + filepath = join(tests_datadir, 'test_site', 'index.html') _, out, _ = yield self.execute([filepath, '-c', 'item']) assert b'{}' in out From 94ee68695a42ec8d3ccdff71bfc2fa33d5ea7049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Thu, 16 Apr 2020 15:36:56 +0200 Subject: [PATCH 25/56] Mock server: use 127.0.0.1 also for HTTPS Windows throws an error about 0.0.0.0 being external: https://stackoverflow.com/a/23857995/939364 --- tests/mockserver.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/mockserver.py b/tests/mockserver.py index a45277db9..e3dbdcc68 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -218,9 +218,8 @@ class MockServer(): self.proc.communicate() def url(self, path, is_secure=False): - host = self.http_address.replace('0.0.0.0', '127.0.0.1') - if is_secure: - host = self.https_address + host = self.https_address if is_secure else self.http_address + host = host.replace('0.0.0.0', '127.0.0.1') return host + path From 7cc9601029274124804e63ebebfa8783ac175205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Thu, 16 Apr 2020 16:57:48 +0200 Subject: [PATCH 26/56] Improve reporting on test_ipv6_alternative_name_resolver --- tests/test_crawler.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 169e763f0..a6b079395 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -309,14 +309,8 @@ class CrawlerProcessSubprocess(unittest.TestCase): def test_ipv6_alternative_name_resolver(self): log = self.run_script('alternative_name_resolver.py') self.assertIn('Spider closed (finished)', log) - self.assertTrue(any([ - "twisted.internet.error.ConnectionRefusedError" in log, - "twisted.internet.error.ConnectError" in log, - ])) - self.assertTrue(any([ - "'downloader/exception_type_count/twisted.internet.error.ConnectionRefusedError': 1," in log, - "'downloader/exception_type_count/twisted.internet.error.ConnectError': 1," in log, - ])) + self.assertRegex(log, r"twisted\.internet\.error\.(?:ConnectionRefusedError|ConnectError)") + self.assertRegex(log, r"'downloader/exception_type_count/twisted\.internet\.error\.(?:ConnectionRefusedError|ConnectError)': 1,") def test_reactor_select(self): log = self.run_script("twisted_reactor_select.py") From cf4180308982d9cd017167238a3df8de7900646d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Thu, 16 Apr 2020 17:07:29 +0200 Subject: [PATCH 27/56] Skip test_reactor_poll on Windows --- tests/test_crawler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index a6b079395..3d166e14c 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -1,5 +1,6 @@ import logging import os +import platform import subprocess import sys import warnings @@ -317,6 +318,7 @@ class CrawlerProcessSubprocess(unittest.TestCase): self.assertIn("Spider closed (finished)", log) self.assertIn("Using reactor: twisted.internet.selectreactor.SelectReactor", log) + @mark.skipif(platform.system() == 'Windows', reason="PollReactor is not supported on Windows") def test_reactor_poll(self): log = self.run_script("twisted_reactor_poll.py") self.assertIn("Spider closed (finished)", log) From ea3e675801fe41c7517c58e21b46831940fbd064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Thu, 16 Apr 2020 17:10:45 +0200 Subject: [PATCH 28/56] test_utils_iterators: use os.linesep --- tests/test_utils_iterators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index 33fc4d570..5b0073fd1 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -8,7 +8,7 @@ from scrapy.http import XmlResponse, TextResponse, Response from tests import get_testdata -FOOBAR_NL = u"foo\nbar" +FOOBAR_NL = "foo{}bar".format(os.linesep) class XmliterTestCase(unittest.TestCase): From 1fecacbb1a3c2c4c61c202561f9c51f7f6b191ba Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Mon, 20 Apr 2020 12:05:15 -0300 Subject: [PATCH 29/56] IPv6 test: check for the absence of DNSLookupError --- tests/test_crawler.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 3d166e14c..d6756c266 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -310,8 +310,7 @@ class CrawlerProcessSubprocess(unittest.TestCase): def test_ipv6_alternative_name_resolver(self): log = self.run_script('alternative_name_resolver.py') self.assertIn('Spider closed (finished)', log) - self.assertRegex(log, r"twisted\.internet\.error\.(?:ConnectionRefusedError|ConnectError)") - self.assertRegex(log, r"'downloader/exception_type_count/twisted\.internet\.error\.(?:ConnectionRefusedError|ConnectError)': 1,") + self.assertNotIn("twisted.internet.error.DNSLookupError", log) def test_reactor_select(self): log = self.run_script("twisted_reactor_select.py") From b99fe4aa4c35ff8f5c2355a09988ddb1e90d6b70 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Fri, 19 Jun 2020 21:41:15 -0300 Subject: [PATCH 30/56] Add google-cloud-storage to the 'pinned' tox environment --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 4c790158d..d28eab131 100644 --- a/tox.ini +++ b/tox.ini @@ -76,6 +76,7 @@ deps = -rtests/requirements-py3.txt # Extras botocore==1.3.23 + google-cloud-storage==1.29.0 Pillow==3.4.2 [testenv:extra-deps] From cfd039aeb6d46f7c120edd82ce06d6b9f4f8e7db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Mon, 22 Jun 2020 19:28:33 +0200 Subject: [PATCH 31/56] Remove a duplicate GCS_PROJECT_ID reference target --- docs/topics/media-pipeline.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst index 01de3dedb..096618648 100644 --- a/docs/topics/media-pipeline.rst +++ b/docs/topics/media-pipeline.rst @@ -204,7 +204,6 @@ For self-hosting you also might feel the need not to use SSL and not to verify S Google Cloud Storage --------------------- -.. setting:: GCS_PROJECT_ID .. setting:: FILES_STORE_GCS_ACL .. setting:: IMAGES_STORE_GCS_ACL From 5b88c522ac1b1a9ba1588573d90cf3bc01339282 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Tue, 30 Jun 2020 12:18:21 -0300 Subject: [PATCH 32/56] Simplify dataclass example in item loader docs --- docs/topics/loaders.rst | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/docs/topics/loaders.rst b/docs/topics/loaders.rst index e921395d2..9c82bb4d9 100644 --- a/docs/topics/loaders.rst +++ b/docs/topics/loaders.rst @@ -88,29 +88,17 @@ item loaders: unless a pre-populated item is passed to the loader, fields will be populated incrementally using the loader's :meth:`~ItemLoader.add_xpath`, :meth:`~ItemLoader.add_css` and :meth:`~ItemLoader.add_value` methods. -Given the way that item loaders store data internally, one approach -to overcome this is to define items using the :func:`~dataclasses.field` -function, with ``list`` as the ``default_factory`` argument:: +One approach to overcome this is to define items using the +:func:`~dataclasses.field` function, with a ``default`` argument:: from dataclasses import dataclass, field + from typing import Optional @dataclass class InventoryItem: - name: str = field(default_factory=list) - price: float = field(default_factory=list) - stock: int = field(default_factory=list) - -Note that in order to keep the example simple, the types do not match -completely. A more accurate but verbose definition would be:: - - from dataclasses import dataclass, field - from typing import List, Union - - @dataclass - class InventoryItem: - name: Union[str, List[str]] = field(default_factory=list) - price: Union[float, List[float]] = field(default_factory=list) - stock: Union[int, List[int]] = field(default_factory=list) + name: Optional[str] = field(default=None) + price: Optional[float] = field(default=None) + stock: Optional[int] = field(default=None) .. _topics-loaders-processors: From af55d23167f9cec22f815bc9f9884b10a9a35f5b Mon Sep 17 00:00:00 2001 From: Andrey Rahmatullin <wrar@wrar.name> Date: Wed, 1 Jul 2020 19:46:54 +0500 Subject: [PATCH 33/56] Update the OpenSSL cipher list format link OpenSSL `ciphers(1)` is now almost empty: https://www.openssl.org/docs/manmaster/man1/ciphers.html Alternative would be linking to 1.1.1 docs specifically. --- docs/topics/settings.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 5178f272f..8cc8806a5 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -469,7 +469,7 @@ necessary to access certain HTTPS websites: for example, you may need to use ``'DEFAULT:!DH'`` for a website with weak DH parameters or enable a specific cipher that is not included in ``DEFAULT`` if a website requires it. -.. _OpenSSL cipher list format: https://www.openssl.org/docs/manmaster/man1/ciphers.html#CIPHER-LIST-FORMAT +.. _OpenSSL cipher list format: https://www.openssl.org/docs/manmaster/man1/openssl-ciphers.html#CIPHER-LIST-FORMAT .. setting:: DOWNLOADER_CLIENT_TLS_METHOD From 3199048520ebe3798c14cfa7362612b51d156b55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Thu, 2 Jul 2020 20:10:08 +0200 Subject: [PATCH 34/56] Complete Azure Pipelines CI setup --- azure-pipelines.yml | 18 ++++++------------ tests/CrawlerRunner/ip_address.py | 15 ++++++++++++++- tests/mockserver.py | 5 ++--- tests/test_commands.py | 5 +++++ tests/test_crawler.py | 13 +++++++++++++ tests/test_feedexport.py | 8 +++++++- tests/test_proxy_connect.py | 3 +++ tests/test_spiderloader/__init__.py | 23 +++++++++++++++-------- tests/test_utils_asyncio.py | 7 ++++++- tox.ini | 18 +++++++++++++++--- 10 files changed, 86 insertions(+), 29 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index ffc4d549b..710e42090 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,29 +1,23 @@ -# Python package -# Create and test a Python package on multiple Python versions. -# Add steps that analyze code, save the dist with the build record, publish to a PyPI-compatible index, and more: -# https://docs.microsoft.com/azure/devops/pipelines/languages/python - - +variables: + TOXENV: py pool: - vmImage: 'windows-2019' + vmImage: 'windows-latest' strategy: matrix: Python35: python.version: '3.5' - TOXENV: py35 + TOXENV: windows-pinned Python36: python.version: '3.6' - TOXENV: py36 Python37: python.version: '3.7' - TOXENV: py37 - + Python38: + python.version: '3.8' steps: - task: UsePythonVersion@0 inputs: versionSpec: '$(python.version)' displayName: 'Use Python $(python.version)' - - script: | pip install -U tox twine wheel codecov tox diff --git a/tests/CrawlerRunner/ip_address.py b/tests/CrawlerRunner/ip_address.py index 826374cd4..ea75bc3c9 100644 --- a/tests/CrawlerRunner/ip_address.py +++ b/tests/CrawlerRunner/ip_address.py @@ -1,7 +1,9 @@ from urllib.parse import urlparse from twisted.internet import reactor -from twisted.names.client import createResolver +from twisted.names import cache, hosts as hostsModule, resolve +from twisted.names.client import Resolver +from twisted.python.runtime import platform from scrapy import Spider, Request from scrapy.crawler import CrawlerRunner @@ -10,6 +12,17 @@ from scrapy.utils.log import configure_logging from tests.mockserver import MockServer, MockDNSServer +# https://stackoverflow.com/a/32784190 +def createResolver(servers=None, resolvconf=None, hosts=None): + if hosts is None: + hosts = (b'/etc/hosts' if platform.getType() == 'posix' + else r'c:\windows\hosts') + theResolver = Resolver(resolvconf, servers) + hostResolver = hostsModule.Resolver(hosts) + L = [hostResolver, cache.CacheResolver(), theResolver] + return resolve.ResolverChain(L) + + class LocalhostSpider(Spider): name = "localhost_spider" diff --git a/tests/mockserver.py b/tests/mockserver.py index df30feab6..1f40473ba 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -247,9 +247,8 @@ class MockDNSServer: def __enter__(self): self.proc = Popen([sys.executable, '-u', '-m', 'tests.mockserver', '-t', 'dns'], stdout=PIPE, env=get_testenv()) - host, port = self.proc.stdout.readline().strip().decode('ascii').split(":") - self.host = host - self.port = int(port) + self.host = '127.0.0.1' + self.port = int(self.proc.stdout.readline().strip().decode('ascii').split(":")[1]) return self def __exit__(self, exc_type, exc_value, traceback): diff --git a/tests/test_commands.py b/tests/test_commands.py index 24a341759..ee0e4511a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2,6 +2,7 @@ import inspect import json import optparse import os +import platform import subprocess import sys import tempfile @@ -10,6 +11,7 @@ from os.path import exists, join, abspath from shutil import rmtree, copytree from tempfile import mkdtemp from threading import Timer +from unittest import skipIf from twisted.trial import unittest @@ -319,6 +321,9 @@ class BadSpider(scrapy.Spider): self.assertIn("start_requests", log) self.assertIn("badspider.py", log) + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") def test_asyncio_enabled_true(self): log = self.get_log(self.debug_log_spider, args=[ '-s', 'TWISTED_REACTOR=twisted.internet.asyncioreactor.AsyncioSelectorReactor' diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 78704fb2c..1a4cfe813 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -4,6 +4,7 @@ import platform import subprocess import sys import warnings +from unittest import skipIf from pytest import raises, mark from testfixtures import LogCapture @@ -252,6 +253,9 @@ class CrawlerRunnerHasSpider(unittest.TestCase): }) @defer.inlineCallbacks + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") def test_crawler_process_asyncio_enabled_true(self): with LogCapture(level=logging.DEBUG) as log: if self.reactor_pytest == 'asyncio': @@ -293,11 +297,17 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): self.assertIn('Spider closed (finished)', log) self.assertNotIn("Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", log) + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") def test_asyncio_enabled_no_reactor(self): log = self.run_script('asyncio_enabled_no_reactor.py') self.assertIn('Spider closed (finished)', log) self.assertIn("Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", log) + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") def test_asyncio_enabled_reactor(self): log = self.run_script('asyncio_enabled_reactor.py') self.assertIn('Spider closed (finished)', log) @@ -327,6 +337,9 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): self.assertIn("Spider closed (finished)", log) self.assertIn("Using reactor: twisted.internet.pollreactor.PollReactor", log) + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") def test_reactor_asyncio(self): log = self.run_script("twisted_reactor_asyncio.py") self.assertIn("Spider closed (finished)", log) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index e38644214..f7b997560 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -455,9 +455,15 @@ class FeedExportTest(unittest.TestCase): def run_and_export(self, spider_cls, settings): """ Run spider with specified settings; return exported data. """ + def path_to_url(path): + return urljoin('file:', pathname2url(str(path))) + + def printf_escape(string): + return string.replace('%', '%%') + FEEDS = settings.get('FEEDS') or {} settings['FEEDS'] = { - urljoin('file:', pathname2url(str(file_path))): feed + printf_escape(path_to_url(file_path)): feed for file_path, feed in FEEDS.items() } diff --git a/tests/test_proxy_connect.py b/tests/test_proxy_connect.py index eb4ecc91d..fc5658ae7 100644 --- a/tests/test_proxy_connect.py +++ b/tests/test_proxy_connect.py @@ -1,5 +1,6 @@ import json import os +import platform import re import sys from subprocess import Popen, PIPE @@ -59,6 +60,8 @@ def _wrong_credentials(proxy_url): @skipIf(sys.version_info < (3, 5, 4), "requires mitmproxy < 3.0.0, which these tests do not support") +@skipIf(platform.system() == 'Windows' and sys.version_info < (3, 7), + "mitmproxy does not support Windows when running Python < 3.7") class ProxyConnectTestCase(TestCase): def setUp(self): diff --git a/tests/test_spiderloader/__init__.py b/tests/test_spiderloader/__init__.py index d922c6059..4929f1e3e 100644 --- a/tests/test_spiderloader/__init__.py +++ b/tests/test_spiderloader/__init__.py @@ -20,13 +20,20 @@ from scrapy.crawler import CrawlerRunner module_dir = os.path.dirname(os.path.abspath(__file__)) +def _copytree(source, target): + try: + shutil.copytree(source, target) + except shutil.Error: + pass + + class SpiderLoaderTest(unittest.TestCase): def setUp(self): orig_spiders_dir = os.path.join(module_dir, 'test_spiders') self.tmpdir = tempfile.mkdtemp() self.spiders_dir = os.path.join(self.tmpdir, 'test_spiders_xxx') - shutil.copytree(orig_spiders_dir, self.spiders_dir) + _copytree(orig_spiders_dir, self.spiders_dir) sys.path.append(self.tmpdir) settings = Settings({'SPIDER_MODULES': ['test_spiders_xxx']}) self.spider_loader = SpiderLoader.from_settings(settings) @@ -124,7 +131,7 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): self.tmpdir = self.mktemp() os.mkdir(self.tmpdir) self.spiders_dir = os.path.join(self.tmpdir, 'test_spiders_xxx') - shutil.copytree(orig_spiders_dir, self.spiders_dir) + _copytree(orig_spiders_dir, self.spiders_dir) sys.path.append(self.tmpdir) self.settings = Settings({'SPIDER_MODULES': ['test_spiders_xxx']}) @@ -134,8 +141,8 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): def test_dupename_warning(self): # copy 1 spider module so as to have duplicate spider name - shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx/spider3.py'), - os.path.join(self.tmpdir, 'test_spiders_xxx/spider3dupe.py')) + shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx', 'spider3.py'), + os.path.join(self.tmpdir, 'test_spiders_xxx', 'spider3dupe.py')) with warnings.catch_warnings(record=True) as w: spider_loader = SpiderLoader.from_settings(self.settings) @@ -156,10 +163,10 @@ class DuplicateSpiderNameLoaderTest(unittest.TestCase): def test_multiple_dupename_warning(self): # copy 2 spider modules so as to have duplicate spider name # This should issue 2 warning, 1 for each duplicate spider name - shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx/spider1.py'), - os.path.join(self.tmpdir, 'test_spiders_xxx/spider1dupe.py')) - shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx/spider2.py'), - os.path.join(self.tmpdir, 'test_spiders_xxx/spider2dupe.py')) + shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx', 'spider1.py'), + os.path.join(self.tmpdir, 'test_spiders_xxx', 'spider1dupe.py')) + shutil.copyfile(os.path.join(self.tmpdir, 'test_spiders_xxx', 'spider2.py'), + os.path.join(self.tmpdir, 'test_spiders_xxx', 'spider2dupe.py')) with warnings.catch_warnings(record=True) as w: spider_loader = SpiderLoader.from_settings(self.settings) diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index 295323e4d..a2114bd18 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -1,4 +1,6 @@ -from unittest import TestCase +import platform +import sys +from unittest import skipIf, TestCase from pytest import mark @@ -12,6 +14,9 @@ class AsyncioTest(TestCase): # the result should depend only on the pytest --reactor argument self.assertEqual(is_asyncio_reactor_installed(), self.reactor_pytest == 'asyncio') + # https://twistedmatrix.com/trac/ticket/9766 + @skipIf(platform.system() == 'Windows' and sys.version_info >= (3, 8), + "the asyncio reactor is broken on Windows when running Python ≥ 3.8") def test_install_asyncio_reactor(self): # this should do nothing install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") diff --git a/tox.ini b/tox.ini index 27d21ade2..f729327ca 100644 --- a/tox.ini +++ b/tox.ini @@ -63,14 +63,12 @@ basepython = pypy3 commands = py.test {posargs:--durations=10 docs scrapy tests} -[testenv:pinned] -basepython = python3 +[pinned] deps = -ctests/constraints.txt cryptography==2.0 cssselect==0.9.1 itemadapter==0.1.0 - lxml==3.5.0 parsel==1.5.0 Protego==0.1.15 PyDispatcher==2.0.5 @@ -85,6 +83,20 @@ deps = botocore==1.3.23 Pillow==3.4.2 +[testenv:pinned] +basepython = python3 +deps = + {[pinned]deps} + lxml==3.5.0 + +[testenv:windows-pinned] +basepython = python3 +deps = + {[pinned]deps} + # First lxml version that includes a Windows wheel for Python 3.5, so we do + # not need to build lxml from sources in a CI Windows job: + lxml==3.8.0 + [testenv:extra-deps] deps = {[testenv]deps} From eb937742566105f3525a9f76e4ae68cc18e9fd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Fri, 3 Jul 2020 01:41:47 +0200 Subject: [PATCH 35/56] TrackrefTestCase.test_get_oldest: protect from lack of precision --- tests/test_utils_trackref.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_utils_trackref.py b/tests/test_utils_trackref.py index 16e02f919..b8e8c3130 100644 --- a/tests/test_utils_trackref.py +++ b/tests/test_utils_trackref.py @@ -1,7 +1,10 @@ import unittest from io import StringIO +from time import sleep, time from unittest import mock +from twisted.trial.unittest import SkipTest + from scrapy.utils import trackref @@ -55,7 +58,18 @@ Foo 1 oldest: 0s ago\n\n''') def test_get_oldest(self): o1 = Foo() # NOQA + + o1_time = time() + o2 = Bar() # NOQA + + o3_time = time() + if o3_time <= o1_time: + sleep(0.01) + o3_time = time() + if o3_time <= o1_time: + raise SkipTest('time.time is not precise enough') + o3 = Foo() # NOQA self.assertIs(trackref.get_oldest('Foo'), o1) self.assertIs(trackref.get_oldest('Bar'), o2) From ec06cf79a6a7264ec3e32d7de8c4e305c2afa05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Mon, 6 Jul 2020 10:47:11 +0200 Subject: [PATCH 36/56] Update tests/CrawlerRunner/ip_address.py Co-authored-by: Eugenio Lacuesta <1731933+elacuesta@users.noreply.github.com> --- tests/CrawlerRunner/ip_address.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/CrawlerRunner/ip_address.py b/tests/CrawlerRunner/ip_address.py index ea75bc3c9..b8254afdf 100644 --- a/tests/CrawlerRunner/ip_address.py +++ b/tests/CrawlerRunner/ip_address.py @@ -15,8 +15,7 @@ from tests.mockserver import MockServer, MockDNSServer # https://stackoverflow.com/a/32784190 def createResolver(servers=None, resolvconf=None, hosts=None): if hosts is None: - hosts = (b'/etc/hosts' if platform.getType() == 'posix' - else r'c:\windows\hosts') + hosts = b'/etc/hosts' if platform.getType() == 'posix' else r'c:\windows\hosts' theResolver = Resolver(resolvconf, servers) hostResolver = hostsModule.Resolver(hosts) L = [hostResolver, cache.CacheResolver(), theResolver] From 17aec5944cab33b3cdcd497d2362cacbf7773e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Mon, 6 Jul 2020 10:47:25 +0200 Subject: [PATCH 37/56] Update tests/CrawlerRunner/ip_address.py Co-authored-by: Eugenio Lacuesta <1731933+elacuesta@users.noreply.github.com> --- tests/CrawlerRunner/ip_address.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/CrawlerRunner/ip_address.py b/tests/CrawlerRunner/ip_address.py index b8254afdf..3f9738798 100644 --- a/tests/CrawlerRunner/ip_address.py +++ b/tests/CrawlerRunner/ip_address.py @@ -18,8 +18,8 @@ def createResolver(servers=None, resolvconf=None, hosts=None): hosts = b'/etc/hosts' if platform.getType() == 'posix' else r'c:\windows\hosts' theResolver = Resolver(resolvconf, servers) hostResolver = hostsModule.Resolver(hosts) - L = [hostResolver, cache.CacheResolver(), theResolver] - return resolve.ResolverChain(L) + chain = [hostResolver, cache.CacheResolver(), theResolver] + return resolve.ResolverChain(chain) class LocalhostSpider(Spider): From 79b4dfc53e431bdad31925aaf16831a0dde536ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Tue, 7 Jul 2020 14:07:04 +0200 Subject: [PATCH 38/56] Fix permission handling on project generation from template files --- scrapy/commands/startproject.py | 30 +--- .../project/module/__init__.py | 0 .../project/module/items.py.tmpl | 12 ++ .../project/module/middlewares.py.tmpl | 103 +++++++++++ .../project/module/pipelines.py.tmpl | 13 ++ .../project/module/settings.py.tmpl | 88 ++++++++++ .../project/module/spiders/__init__.py | 4 + .../read_only_templates/project/scrapy.cfg | 11 ++ .../read_only_templates/spiders/basic.tmpl | 10 ++ .../read_only_templates/spiders/crawl.tmpl | 20 +++ .../read_only_templates/spiders/csvfeed.tmpl | 20 +++ .../read_only_templates/spiders/xmlfeed.tmpl | 16 ++ tests/test_commands.py | 164 ++++++++++++++++++ 13 files changed, 467 insertions(+), 24 deletions(-) create mode 100644 tests/sample_data/read_only_templates/project/module/__init__.py create mode 100644 tests/sample_data/read_only_templates/project/module/items.py.tmpl create mode 100644 tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl create mode 100644 tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl create mode 100644 tests/sample_data/read_only_templates/project/module/settings.py.tmpl create mode 100644 tests/sample_data/read_only_templates/project/module/spiders/__init__.py create mode 100644 tests/sample_data/read_only_templates/project/scrapy.cfg create mode 100644 tests/sample_data/read_only_templates/spiders/basic.tmpl create mode 100644 tests/sample_data/read_only_templates/spiders/crawl.tmpl create mode 100644 tests/sample_data/read_only_templates/spiders/csvfeed.tmpl create mode 100644 tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl diff --git a/scrapy/commands/startproject.py b/scrapy/commands/startproject.py index 852281959..e702d7cdc 100644 --- a/scrapy/commands/startproject.py +++ b/scrapy/commands/startproject.py @@ -1,10 +1,10 @@ import re import os -import stat import string from importlib import import_module from os.path import join, exists, abspath from shutil import ignore_patterns, move, copy2, copystat +from stat import S_IWUSR as OWNER_WRITE_PERMISSION import scrapy from scrapy.commands import ScrapyCommand @@ -78,30 +78,12 @@ class Command(ScrapyCommand): self._copytree(srcname, dstname) else: copy2(srcname, dstname) + current_permissions = os.stat(dstname).st_mode + os.chmod(dstname, current_permissions | OWNER_WRITE_PERMISSION) + copystat(src, dst) - self._set_rw_permissions(dst) - - def _set_rw_permissions(self, path): - """ - Sets permissions of a directory tree to +rw and +rwx for folders. - This is necessary if the start template files come without write - permissions. - """ - mode_rw = (stat.S_IRUSR - | stat.S_IWUSR - | stat.S_IRGRP - | stat.S_IROTH) - - mode_x = (stat.S_IXUSR - | stat.S_IXGRP - | stat.S_IXOTH) - - os.chmod(path, mode_rw | mode_x) - for root, dirs, files in os.walk(path): - for dir in dirs: - os.chmod(join(root, dir), mode_rw | mode_x) - for file in files: - os.chmod(join(root, file), mode_rw) + current_permissions = os.stat(dst).st_mode + os.chmod(dst, current_permissions | OWNER_WRITE_PERMISSION) def run(self, args, opts): if len(args) not in (1, 2): diff --git a/tests/sample_data/read_only_templates/project/module/__init__.py b/tests/sample_data/read_only_templates/project/module/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sample_data/read_only_templates/project/module/items.py.tmpl b/tests/sample_data/read_only_templates/project/module/items.py.tmpl new file mode 100644 index 000000000..88a18331c --- /dev/null +++ b/tests/sample_data/read_only_templates/project/module/items.py.tmpl @@ -0,0 +1,12 @@ +# Define here the models for your scraped items +# +# See documentation in: +# https://docs.scrapy.org/en/latest/topics/items.html + +import scrapy + + +class ${ProjectName}Item(scrapy.Item): + # define the fields for your item here like: + # name = scrapy.Field() + pass diff --git a/tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl b/tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl new file mode 100644 index 000000000..bd09890fe --- /dev/null +++ b/tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl @@ -0,0 +1,103 @@ +# Define here the models for your spider middleware +# +# See documentation in: +# https://docs.scrapy.org/en/latest/topics/spider-middleware.html + +from scrapy import signals + +# useful for handling different item types with a single interface +from itemadapter import is_item, ItemAdapter + + +class ${ProjectName}SpiderMiddleware: + # Not all methods need to be defined. If a method is not defined, + # scrapy acts as if the spider middleware does not modify the + # passed objects. + + @classmethod + def from_crawler(cls, crawler): + # This method is used by Scrapy to create your spiders. + s = cls() + crawler.signals.connect(s.spider_opened, signal=signals.spider_opened) + return s + + def process_spider_input(self, response, spider): + # Called for each response that goes through the spider + # middleware and into the spider. + + # Should return None or raise an exception. + return None + + def process_spider_output(self, response, result, spider): + # Called with the results returned from the Spider, after + # it has processed the response. + + # Must return an iterable of Request, or item objects. + for i in result: + yield i + + def process_spider_exception(self, response, exception, spider): + # Called when a spider or process_spider_input() method + # (from other spider middleware) raises an exception. + + # Should return either None or an iterable of Request or item objects. + pass + + def process_start_requests(self, start_requests, spider): + # Called with the start requests of the spider, and works + # similarly to the process_spider_output() method, except + # that it doesn’t have a response associated. + + # Must return only requests (not items). + for r in start_requests: + yield r + + def spider_opened(self, spider): + spider.logger.info('Spider opened: %s' % spider.name) + + +class ${ProjectName}DownloaderMiddleware: + # Not all methods need to be defined. If a method is not defined, + # scrapy acts as if the downloader middleware does not modify the + # passed objects. + + @classmethod + def from_crawler(cls, crawler): + # This method is used by Scrapy to create your spiders. + s = cls() + crawler.signals.connect(s.spider_opened, signal=signals.spider_opened) + return s + + def process_request(self, request, spider): + # Called for each request that goes through the downloader + # middleware. + + # Must either: + # - return None: continue processing this request + # - or return a Response object + # - or return a Request object + # - or raise IgnoreRequest: process_exception() methods of + # installed downloader middleware will be called + return None + + def process_response(self, request, response, spider): + # Called with the response returned from the downloader. + + # Must either; + # - return a Response object + # - return a Request object + # - or raise IgnoreRequest + return response + + def process_exception(self, request, exception, spider): + # Called when a download handler or a process_request() + # (from other downloader middleware) raises an exception. + + # Must either: + # - return None: continue processing this exception + # - return a Response object: stops process_exception() chain + # - return a Request object: stops process_exception() chain + pass + + def spider_opened(self, spider): + spider.logger.info('Spider opened: %s' % spider.name) diff --git a/tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl b/tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl new file mode 100644 index 000000000..e845f43e9 --- /dev/null +++ b/tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl @@ -0,0 +1,13 @@ +# Define your item pipelines here +# +# Don't forget to add your pipeline to the ITEM_PIPELINES setting +# See: https://docs.scrapy.org/en/latest/topics/item-pipeline.html + + +# useful for handling different item types with a single interface +from itemadapter import ItemAdapter + + +class ${ProjectName}Pipeline: + def process_item(self, item, spider): + return item diff --git a/tests/sample_data/read_only_templates/project/module/settings.py.tmpl b/tests/sample_data/read_only_templates/project/module/settings.py.tmpl new file mode 100644 index 000000000..a414b5fde --- /dev/null +++ b/tests/sample_data/read_only_templates/project/module/settings.py.tmpl @@ -0,0 +1,88 @@ +# Scrapy settings for $project_name project +# +# For simplicity, this file contains only settings considered important or +# commonly used. You can find more settings consulting the documentation: +# +# https://docs.scrapy.org/en/latest/topics/settings.html +# https://docs.scrapy.org/en/latest/topics/downloader-middleware.html +# https://docs.scrapy.org/en/latest/topics/spider-middleware.html + +BOT_NAME = '$project_name' + +SPIDER_MODULES = ['$project_name.spiders'] +NEWSPIDER_MODULE = '$project_name.spiders' + + +# Crawl responsibly by identifying yourself (and your website) on the user-agent +#USER_AGENT = '$project_name (+http://www.yourdomain.com)' + +# Obey robots.txt rules +ROBOTSTXT_OBEY = True + +# Configure maximum concurrent requests performed by Scrapy (default: 16) +#CONCURRENT_REQUESTS = 32 + +# Configure a delay for requests for the same website (default: 0) +# See https://docs.scrapy.org/en/latest/topics/settings.html#download-delay +# See also autothrottle settings and docs +#DOWNLOAD_DELAY = 3 +# The download delay setting will honor only one of: +#CONCURRENT_REQUESTS_PER_DOMAIN = 16 +#CONCURRENT_REQUESTS_PER_IP = 16 + +# Disable cookies (enabled by default) +#COOKIES_ENABLED = False + +# Disable Telnet Console (enabled by default) +#TELNETCONSOLE_ENABLED = False + +# Override the default request headers: +#DEFAULT_REQUEST_HEADERS = { +# 'Accept': 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', +# 'Accept-Language': 'en', +#} + +# Enable or disable spider middlewares +# See https://docs.scrapy.org/en/latest/topics/spider-middleware.html +#SPIDER_MIDDLEWARES = { +# '$project_name.middlewares.${ProjectName}SpiderMiddleware': 543, +#} + +# Enable or disable downloader middlewares +# See https://docs.scrapy.org/en/latest/topics/downloader-middleware.html +#DOWNLOADER_MIDDLEWARES = { +# '$project_name.middlewares.${ProjectName}DownloaderMiddleware': 543, +#} + +# Enable or disable extensions +# See https://docs.scrapy.org/en/latest/topics/extensions.html +#EXTENSIONS = { +# 'scrapy.extensions.telnet.TelnetConsole': None, +#} + +# Configure item pipelines +# See https://docs.scrapy.org/en/latest/topics/item-pipeline.html +#ITEM_PIPELINES = { +# '$project_name.pipelines.${ProjectName}Pipeline': 300, +#} + +# Enable and configure the AutoThrottle extension (disabled by default) +# See https://docs.scrapy.org/en/latest/topics/autothrottle.html +#AUTOTHROTTLE_ENABLED = True +# The initial download delay +#AUTOTHROTTLE_START_DELAY = 5 +# The maximum download delay to be set in case of high latencies +#AUTOTHROTTLE_MAX_DELAY = 60 +# The average number of requests Scrapy should be sending in parallel to +# each remote server +#AUTOTHROTTLE_TARGET_CONCURRENCY = 1.0 +# Enable showing throttling stats for every response received: +#AUTOTHROTTLE_DEBUG = False + +# Enable and configure HTTP caching (disabled by default) +# See https://docs.scrapy.org/en/latest/topics/downloader-middleware.html#httpcache-middleware-settings +#HTTPCACHE_ENABLED = True +#HTTPCACHE_EXPIRATION_SECS = 0 +#HTTPCACHE_DIR = 'httpcache' +#HTTPCACHE_IGNORE_HTTP_CODES = [] +#HTTPCACHE_STORAGE = 'scrapy.extensions.httpcache.FilesystemCacheStorage' diff --git a/tests/sample_data/read_only_templates/project/module/spiders/__init__.py b/tests/sample_data/read_only_templates/project/module/spiders/__init__.py new file mode 100644 index 000000000..ebd689ac5 --- /dev/null +++ b/tests/sample_data/read_only_templates/project/module/spiders/__init__.py @@ -0,0 +1,4 @@ +# This package will contain the spiders of your Scrapy project +# +# Please refer to the documentation for information on how to create and manage +# your spiders. diff --git a/tests/sample_data/read_only_templates/project/scrapy.cfg b/tests/sample_data/read_only_templates/project/scrapy.cfg new file mode 100644 index 000000000..1daeaa541 --- /dev/null +++ b/tests/sample_data/read_only_templates/project/scrapy.cfg @@ -0,0 +1,11 @@ +# Automatically created by: scrapy startproject +# +# For more information about the [deploy] section see: +# https://scrapyd.readthedocs.io/en/latest/deploy.html + +[settings] +default = ${project_name}.settings + +[deploy] +#url = http://localhost:6800/ +project = ${project_name} diff --git a/tests/sample_data/read_only_templates/spiders/basic.tmpl b/tests/sample_data/read_only_templates/spiders/basic.tmpl new file mode 100644 index 000000000..e9112bc95 --- /dev/null +++ b/tests/sample_data/read_only_templates/spiders/basic.tmpl @@ -0,0 +1,10 @@ +import scrapy + + +class $classname(scrapy.Spider): + name = '$name' + allowed_domains = ['$domain'] + start_urls = ['http://$domain/'] + + def parse(self, response): + pass diff --git a/tests/sample_data/read_only_templates/spiders/crawl.tmpl b/tests/sample_data/read_only_templates/spiders/crawl.tmpl new file mode 100644 index 000000000..356496487 --- /dev/null +++ b/tests/sample_data/read_only_templates/spiders/crawl.tmpl @@ -0,0 +1,20 @@ +import scrapy +from scrapy.linkextractors import LinkExtractor +from scrapy.spiders import CrawlSpider, Rule + + +class $classname(CrawlSpider): + name = '$name' + allowed_domains = ['$domain'] + start_urls = ['http://$domain/'] + + rules = ( + Rule(LinkExtractor(allow=r'Items/'), callback='parse_item', follow=True), + ) + + def parse_item(self, response): + item = {} + #item['domain_id'] = response.xpath('//input[@id="sid"]/@value').get() + #item['name'] = response.xpath('//div[@id="name"]').get() + #item['description'] = response.xpath('//div[@id="description"]').get() + return item diff --git a/tests/sample_data/read_only_templates/spiders/csvfeed.tmpl b/tests/sample_data/read_only_templates/spiders/csvfeed.tmpl new file mode 100644 index 000000000..cbcbe9e2c --- /dev/null +++ b/tests/sample_data/read_only_templates/spiders/csvfeed.tmpl @@ -0,0 +1,20 @@ +from scrapy.spiders import CSVFeedSpider + + +class $classname(CSVFeedSpider): + name = '$name' + allowed_domains = ['$domain'] + start_urls = ['http://$domain/feed.csv'] + # headers = ['id', 'name', 'description', 'image_link'] + # delimiter = '\t' + + # Do any adaptations you need here + #def adapt_response(self, response): + # return response + + def parse_row(self, response, row): + i = {} + #i['url'] = row['url'] + #i['name'] = row['name'] + #i['description'] = row['description'] + return i diff --git a/tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl b/tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl new file mode 100644 index 000000000..5aa2aa8b0 --- /dev/null +++ b/tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl @@ -0,0 +1,16 @@ +from scrapy.spiders import XMLFeedSpider + + +class $classname(XMLFeedSpider): + name = '$name' + allowed_domains = ['$domain'] + start_urls = ['http://$domain/feed.xml'] + iterator = 'iternodes' # you can change this; see the docs + itertag = 'item' # change it accordingly + + def parse_node(self, response, selector): + item = {} + #item['url'] = selector.select('url').get() + #item['name'] = selector.select('name').get() + #item['description'] = selector.select('description').get() + return item diff --git a/tests/test_commands.py b/tests/test_commands.py index 24a341759..8336c8759 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -6,7 +6,9 @@ import subprocess import sys import tempfile from contextlib import contextmanager +from itertools import chain from os.path import exists, join, abspath +from pathlib import Path from shutil import rmtree, copytree from tempfile import mkdtemp from threading import Timer @@ -15,6 +17,7 @@ from twisted.trial import unittest import scrapy from scrapy.commands import ScrapyCommand +from scrapy.commands.startproject import IGNORE from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import get_testenv @@ -119,8 +122,34 @@ class StartprojectTest(ProjectTest): self.assertEqual(2, self.call('startproject', self.project_name, project_dir, 'another_params')) +def get_permissions_dict(path, renamings=None, ignore=None): + renamings = renamings or tuple() + permissions_dict = { + '.': os.stat(path).st_mode, + } + for root, dirs, files in os.walk(path): + nodes = list(chain(dirs, files)) + if ignore: + ignored_names = ignore(root, nodes) + nodes = [node for node in nodes + if node not in ignored_names] + for node in nodes: + absolute_path = os.path.join(root, node) + relative_path = os.path.relpath(absolute_path, path) + for search_string, replacement in renamings: + relative_path = relative_path.replace( + search_string, + replacement + ) + permissions = os.stat(absolute_path).st_mode + permissions_dict[relative_path] = permissions + return permissions_dict + + class StartprojectTemplatesTest(ProjectTest): + maxDiff = None + def setUp(self): super(StartprojectTemplatesTest, self).setUp() self.tmpl = join(self.temp_path, 'templates') @@ -139,6 +168,141 @@ class StartprojectTemplatesTest(ProjectTest): self.assertIn(self.tmpl_proj, out) assert exists(join(self.proj_path, 'root_template')) + def test_startproject_permissions_from_writable(self): + """Check that generated files have the right permissions when the + template folder has the same permissions as in the project, i.e. + everything is writable.""" + scrapy_path = scrapy.__path__[0] + templates_dir = os.path.join(scrapy_path, 'templates', 'project') + project_name = 'startproject1' + renamings = ( + ('module', project_name), + ('.tmpl', ''), + ) + expected_permissions = get_permissions_dict( + templates_dir, + renamings, + IGNORE, + ) + + destination = mkdtemp() + process = subprocess.Popen( + ( + sys.executable, + '-m', + 'scrapy.cmdline', + 'startproject', + project_name, + ), + cwd=destination, + env=self.env, + ) + process.wait() + + project_dir = os.path.join(destination, project_name) + actual_permissions = get_permissions_dict(project_dir) + + self.assertEqual(actual_permissions, expected_permissions) + + def test_startproject_permissions_from_read_only(self): + """Check that generated files have the right permissions when the + template folder has been made read-only, which is something that some + systems do. + + See https://github.com/scrapy/scrapy/pull/4604 + """ + scrapy_path = scrapy.__path__[0] + templates_dir = os.path.join(scrapy_path, 'templates', 'project') + project_name = 'startproject2' + renamings = ( + ('module', project_name), + ('.tmpl', ''), + ) + expected_permissions = get_permissions_dict( + templates_dir, + renamings, + IGNORE, + ) + + tests_path = os.path.dirname(__file__) + read_only_templates_dir = os.path.join( + tests_path, 'sample_data', 'read_only_templates' + ) + destination = mkdtemp() + process = subprocess.Popen( + ( + sys.executable, + '-m', + 'scrapy.cmdline', + 'startproject', + project_name, + '--set', + 'TEMPLATES_DIR={}'.format(read_only_templates_dir), + ), + cwd=destination, + env=self.env, + ) + process.wait() + + project_dir = os.path.join(destination, project_name) + actual_permissions = get_permissions_dict(project_dir) + + self.assertEqual(actual_permissions, expected_permissions) + + def test_startproject_permissions_unchanged_in_destination(self): + """Check that pre-existing folders and files in the destination folder + do not see their permissions modified.""" + scrapy_path = scrapy.__path__[0] + templates_dir = os.path.join(scrapy_path, 'templates', 'project') + project_name = 'startproject3' + renamings = ( + ('module', project_name), + ('.tmpl', ''), + ) + expected_permissions = get_permissions_dict( + templates_dir, + renamings, + IGNORE, + ) + + destination = mkdtemp() + project_dir = os.path.join(destination, project_name) + + existing_nodes = { + oct(permissions)[2:] + extension: permissions + for extension in ('', '.d') + for permissions in ( + 0o444, 0o555, 0o644, 0o666, 0o755, 0o777, + ) + } + os.mkdir(project_dir) + project_dir_path = Path(project_dir) + for node, permissions in existing_nodes.items(): + path = project_dir_path / node + if node.endswith('.d'): + path.mkdir(mode=permissions) + else: + path.touch(mode=permissions) + expected_permissions[node] = path.stat().st_mode + + process = subprocess.Popen( + ( + sys.executable, + '-m', + 'scrapy.cmdline', + 'startproject', + project_name, + '.', + ), + cwd=project_dir, + env=self.env, + ) + process.wait() + + actual_permissions = get_permissions_dict(project_dir) + + self.assertEqual(actual_permissions, expected_permissions) + class CommandTest(ProjectTest): From a3afff4a0e1b25903d1de5c5501846d1f1288d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Tue, 7 Jul 2020 14:11:02 +0200 Subject: [PATCH 39/56] Fix style issue --- tests/test_commands.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index 8336c8759..bd799817d 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -131,8 +131,7 @@ def get_permissions_dict(path, renamings=None, ignore=None): nodes = list(chain(dirs, files)) if ignore: ignored_names = ignore(root, nodes) - nodes = [node for node in nodes - if node not in ignored_names] + nodes = [node for node in nodes if node not in ignored_names] for node in nodes: absolute_path = os.path.join(root, node) relative_path = os.path.relpath(absolute_path, path) From e1450799ce2dfa32e248cb0b4069668ee5e6f4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Tue, 7 Jul 2020 14:11:37 +0200 Subject: [PATCH 40/56] Remove debug test case variable --- tests/test_commands.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index bd799817d..c25495d16 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -147,8 +147,6 @@ def get_permissions_dict(path, renamings=None, ignore=None): class StartprojectTemplatesTest(ProjectTest): - maxDiff = None - def setUp(self): super(StartprojectTemplatesTest, self).setUp() self.tmpl = join(self.temp_path, 'templates') From ca77ca1f751a614b0c9394d1e8c6d0b9cfcdd957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Tue, 7 Jul 2020 14:44:03 +0200 Subject: [PATCH 41/56] Generate read-only files on the fly --- scrapy/commands/startproject.py | 13 ++- .../project/module/__init__.py | 0 .../project/module/items.py.tmpl | 12 -- .../project/module/middlewares.py.tmpl | 103 ------------------ .../project/module/pipelines.py.tmpl | 13 --- .../project/module/settings.py.tmpl | 88 --------------- .../project/module/spiders/__init__.py | 4 - .../read_only_templates/project/scrapy.cfg | 11 -- .../read_only_templates/spiders/basic.tmpl | 10 -- .../read_only_templates/spiders/crawl.tmpl | 20 ---- .../read_only_templates/spiders/csvfeed.tmpl | 20 ---- .../read_only_templates/spiders/xmlfeed.tmpl | 16 --- tests/test_commands.py | 31 ++++-- 13 files changed, 28 insertions(+), 313 deletions(-) delete mode 100644 tests/sample_data/read_only_templates/project/module/__init__.py delete mode 100644 tests/sample_data/read_only_templates/project/module/items.py.tmpl delete mode 100644 tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl delete mode 100644 tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl delete mode 100644 tests/sample_data/read_only_templates/project/module/settings.py.tmpl delete mode 100644 tests/sample_data/read_only_templates/project/module/spiders/__init__.py delete mode 100644 tests/sample_data/read_only_templates/project/scrapy.cfg delete mode 100644 tests/sample_data/read_only_templates/spiders/basic.tmpl delete mode 100644 tests/sample_data/read_only_templates/spiders/crawl.tmpl delete mode 100644 tests/sample_data/read_only_templates/spiders/csvfeed.tmpl delete mode 100644 tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl diff --git a/scrapy/commands/startproject.py b/scrapy/commands/startproject.py index e702d7cdc..eccc2a3e1 100644 --- a/scrapy/commands/startproject.py +++ b/scrapy/commands/startproject.py @@ -20,7 +20,12 @@ TEMPLATES_TO_RENDER = ( ('${project_name}', 'middlewares.py.tmpl'), ) -IGNORE = ignore_patterns('*.pyc', '.svn') +IGNORE = ignore_patterns('*.pyc', '__pycache__', '.svn') + + +def _make_writable(path): + current_permissions = os.stat(path).st_mode + os.chmod(path, current_permissions | OWNER_WRITE_PERMISSION) class Command(ScrapyCommand): @@ -78,12 +83,10 @@ class Command(ScrapyCommand): self._copytree(srcname, dstname) else: copy2(srcname, dstname) - current_permissions = os.stat(dstname).st_mode - os.chmod(dstname, current_permissions | OWNER_WRITE_PERMISSION) + _make_writable(dstname) copystat(src, dst) - current_permissions = os.stat(dst).st_mode - os.chmod(dst, current_permissions | OWNER_WRITE_PERMISSION) + _make_writable(dst) def run(self, args, opts): if len(args) not in (1, 2): diff --git a/tests/sample_data/read_only_templates/project/module/__init__.py b/tests/sample_data/read_only_templates/project/module/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/sample_data/read_only_templates/project/module/items.py.tmpl b/tests/sample_data/read_only_templates/project/module/items.py.tmpl deleted file mode 100644 index 88a18331c..000000000 --- a/tests/sample_data/read_only_templates/project/module/items.py.tmpl +++ /dev/null @@ -1,12 +0,0 @@ -# Define here the models for your scraped items -# -# See documentation in: -# https://docs.scrapy.org/en/latest/topics/items.html - -import scrapy - - -class ${ProjectName}Item(scrapy.Item): - # define the fields for your item here like: - # name = scrapy.Field() - pass diff --git a/tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl b/tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl deleted file mode 100644 index bd09890fe..000000000 --- a/tests/sample_data/read_only_templates/project/module/middlewares.py.tmpl +++ /dev/null @@ -1,103 +0,0 @@ -# Define here the models for your spider middleware -# -# See documentation in: -# https://docs.scrapy.org/en/latest/topics/spider-middleware.html - -from scrapy import signals - -# useful for handling different item types with a single interface -from itemadapter import is_item, ItemAdapter - - -class ${ProjectName}SpiderMiddleware: - # Not all methods need to be defined. If a method is not defined, - # scrapy acts as if the spider middleware does not modify the - # passed objects. - - @classmethod - def from_crawler(cls, crawler): - # This method is used by Scrapy to create your spiders. - s = cls() - crawler.signals.connect(s.spider_opened, signal=signals.spider_opened) - return s - - def process_spider_input(self, response, spider): - # Called for each response that goes through the spider - # middleware and into the spider. - - # Should return None or raise an exception. - return None - - def process_spider_output(self, response, result, spider): - # Called with the results returned from the Spider, after - # it has processed the response. - - # Must return an iterable of Request, or item objects. - for i in result: - yield i - - def process_spider_exception(self, response, exception, spider): - # Called when a spider or process_spider_input() method - # (from other spider middleware) raises an exception. - - # Should return either None or an iterable of Request or item objects. - pass - - def process_start_requests(self, start_requests, spider): - # Called with the start requests of the spider, and works - # similarly to the process_spider_output() method, except - # that it doesn’t have a response associated. - - # Must return only requests (not items). - for r in start_requests: - yield r - - def spider_opened(self, spider): - spider.logger.info('Spider opened: %s' % spider.name) - - -class ${ProjectName}DownloaderMiddleware: - # Not all methods need to be defined. If a method is not defined, - # scrapy acts as if the downloader middleware does not modify the - # passed objects. - - @classmethod - def from_crawler(cls, crawler): - # This method is used by Scrapy to create your spiders. - s = cls() - crawler.signals.connect(s.spider_opened, signal=signals.spider_opened) - return s - - def process_request(self, request, spider): - # Called for each request that goes through the downloader - # middleware. - - # Must either: - # - return None: continue processing this request - # - or return a Response object - # - or return a Request object - # - or raise IgnoreRequest: process_exception() methods of - # installed downloader middleware will be called - return None - - def process_response(self, request, response, spider): - # Called with the response returned from the downloader. - - # Must either; - # - return a Response object - # - return a Request object - # - or raise IgnoreRequest - return response - - def process_exception(self, request, exception, spider): - # Called when a download handler or a process_request() - # (from other downloader middleware) raises an exception. - - # Must either: - # - return None: continue processing this exception - # - return a Response object: stops process_exception() chain - # - return a Request object: stops process_exception() chain - pass - - def spider_opened(self, spider): - spider.logger.info('Spider opened: %s' % spider.name) diff --git a/tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl b/tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl deleted file mode 100644 index e845f43e9..000000000 --- a/tests/sample_data/read_only_templates/project/module/pipelines.py.tmpl +++ /dev/null @@ -1,13 +0,0 @@ -# Define your item pipelines here -# -# Don't forget to add your pipeline to the ITEM_PIPELINES setting -# See: https://docs.scrapy.org/en/latest/topics/item-pipeline.html - - -# useful for handling different item types with a single interface -from itemadapter import ItemAdapter - - -class ${ProjectName}Pipeline: - def process_item(self, item, spider): - return item diff --git a/tests/sample_data/read_only_templates/project/module/settings.py.tmpl b/tests/sample_data/read_only_templates/project/module/settings.py.tmpl deleted file mode 100644 index a414b5fde..000000000 --- a/tests/sample_data/read_only_templates/project/module/settings.py.tmpl +++ /dev/null @@ -1,88 +0,0 @@ -# Scrapy settings for $project_name project -# -# For simplicity, this file contains only settings considered important or -# commonly used. You can find more settings consulting the documentation: -# -# https://docs.scrapy.org/en/latest/topics/settings.html -# https://docs.scrapy.org/en/latest/topics/downloader-middleware.html -# https://docs.scrapy.org/en/latest/topics/spider-middleware.html - -BOT_NAME = '$project_name' - -SPIDER_MODULES = ['$project_name.spiders'] -NEWSPIDER_MODULE = '$project_name.spiders' - - -# Crawl responsibly by identifying yourself (and your website) on the user-agent -#USER_AGENT = '$project_name (+http://www.yourdomain.com)' - -# Obey robots.txt rules -ROBOTSTXT_OBEY = True - -# Configure maximum concurrent requests performed by Scrapy (default: 16) -#CONCURRENT_REQUESTS = 32 - -# Configure a delay for requests for the same website (default: 0) -# See https://docs.scrapy.org/en/latest/topics/settings.html#download-delay -# See also autothrottle settings and docs -#DOWNLOAD_DELAY = 3 -# The download delay setting will honor only one of: -#CONCURRENT_REQUESTS_PER_DOMAIN = 16 -#CONCURRENT_REQUESTS_PER_IP = 16 - -# Disable cookies (enabled by default) -#COOKIES_ENABLED = False - -# Disable Telnet Console (enabled by default) -#TELNETCONSOLE_ENABLED = False - -# Override the default request headers: -#DEFAULT_REQUEST_HEADERS = { -# 'Accept': 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8', -# 'Accept-Language': 'en', -#} - -# Enable or disable spider middlewares -# See https://docs.scrapy.org/en/latest/topics/spider-middleware.html -#SPIDER_MIDDLEWARES = { -# '$project_name.middlewares.${ProjectName}SpiderMiddleware': 543, -#} - -# Enable or disable downloader middlewares -# See https://docs.scrapy.org/en/latest/topics/downloader-middleware.html -#DOWNLOADER_MIDDLEWARES = { -# '$project_name.middlewares.${ProjectName}DownloaderMiddleware': 543, -#} - -# Enable or disable extensions -# See https://docs.scrapy.org/en/latest/topics/extensions.html -#EXTENSIONS = { -# 'scrapy.extensions.telnet.TelnetConsole': None, -#} - -# Configure item pipelines -# See https://docs.scrapy.org/en/latest/topics/item-pipeline.html -#ITEM_PIPELINES = { -# '$project_name.pipelines.${ProjectName}Pipeline': 300, -#} - -# Enable and configure the AutoThrottle extension (disabled by default) -# See https://docs.scrapy.org/en/latest/topics/autothrottle.html -#AUTOTHROTTLE_ENABLED = True -# The initial download delay -#AUTOTHROTTLE_START_DELAY = 5 -# The maximum download delay to be set in case of high latencies -#AUTOTHROTTLE_MAX_DELAY = 60 -# The average number of requests Scrapy should be sending in parallel to -# each remote server -#AUTOTHROTTLE_TARGET_CONCURRENCY = 1.0 -# Enable showing throttling stats for every response received: -#AUTOTHROTTLE_DEBUG = False - -# Enable and configure HTTP caching (disabled by default) -# See https://docs.scrapy.org/en/latest/topics/downloader-middleware.html#httpcache-middleware-settings -#HTTPCACHE_ENABLED = True -#HTTPCACHE_EXPIRATION_SECS = 0 -#HTTPCACHE_DIR = 'httpcache' -#HTTPCACHE_IGNORE_HTTP_CODES = [] -#HTTPCACHE_STORAGE = 'scrapy.extensions.httpcache.FilesystemCacheStorage' diff --git a/tests/sample_data/read_only_templates/project/module/spiders/__init__.py b/tests/sample_data/read_only_templates/project/module/spiders/__init__.py deleted file mode 100644 index ebd689ac5..000000000 --- a/tests/sample_data/read_only_templates/project/module/spiders/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -# This package will contain the spiders of your Scrapy project -# -# Please refer to the documentation for information on how to create and manage -# your spiders. diff --git a/tests/sample_data/read_only_templates/project/scrapy.cfg b/tests/sample_data/read_only_templates/project/scrapy.cfg deleted file mode 100644 index 1daeaa541..000000000 --- a/tests/sample_data/read_only_templates/project/scrapy.cfg +++ /dev/null @@ -1,11 +0,0 @@ -# Automatically created by: scrapy startproject -# -# For more information about the [deploy] section see: -# https://scrapyd.readthedocs.io/en/latest/deploy.html - -[settings] -default = ${project_name}.settings - -[deploy] -#url = http://localhost:6800/ -project = ${project_name} diff --git a/tests/sample_data/read_only_templates/spiders/basic.tmpl b/tests/sample_data/read_only_templates/spiders/basic.tmpl deleted file mode 100644 index e9112bc95..000000000 --- a/tests/sample_data/read_only_templates/spiders/basic.tmpl +++ /dev/null @@ -1,10 +0,0 @@ -import scrapy - - -class $classname(scrapy.Spider): - name = '$name' - allowed_domains = ['$domain'] - start_urls = ['http://$domain/'] - - def parse(self, response): - pass diff --git a/tests/sample_data/read_only_templates/spiders/crawl.tmpl b/tests/sample_data/read_only_templates/spiders/crawl.tmpl deleted file mode 100644 index 356496487..000000000 --- a/tests/sample_data/read_only_templates/spiders/crawl.tmpl +++ /dev/null @@ -1,20 +0,0 @@ -import scrapy -from scrapy.linkextractors import LinkExtractor -from scrapy.spiders import CrawlSpider, Rule - - -class $classname(CrawlSpider): - name = '$name' - allowed_domains = ['$domain'] - start_urls = ['http://$domain/'] - - rules = ( - Rule(LinkExtractor(allow=r'Items/'), callback='parse_item', follow=True), - ) - - def parse_item(self, response): - item = {} - #item['domain_id'] = response.xpath('//input[@id="sid"]/@value').get() - #item['name'] = response.xpath('//div[@id="name"]').get() - #item['description'] = response.xpath('//div[@id="description"]').get() - return item diff --git a/tests/sample_data/read_only_templates/spiders/csvfeed.tmpl b/tests/sample_data/read_only_templates/spiders/csvfeed.tmpl deleted file mode 100644 index cbcbe9e2c..000000000 --- a/tests/sample_data/read_only_templates/spiders/csvfeed.tmpl +++ /dev/null @@ -1,20 +0,0 @@ -from scrapy.spiders import CSVFeedSpider - - -class $classname(CSVFeedSpider): - name = '$name' - allowed_domains = ['$domain'] - start_urls = ['http://$domain/feed.csv'] - # headers = ['id', 'name', 'description', 'image_link'] - # delimiter = '\t' - - # Do any adaptations you need here - #def adapt_response(self, response): - # return response - - def parse_row(self, response, row): - i = {} - #i['url'] = row['url'] - #i['name'] = row['name'] - #i['description'] = row['description'] - return i diff --git a/tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl b/tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl deleted file mode 100644 index 5aa2aa8b0..000000000 --- a/tests/sample_data/read_only_templates/spiders/xmlfeed.tmpl +++ /dev/null @@ -1,16 +0,0 @@ -from scrapy.spiders import XMLFeedSpider - - -class $classname(XMLFeedSpider): - name = '$name' - allowed_domains = ['$domain'] - start_urls = ['http://$domain/feed.xml'] - iterator = 'iternodes' # you can change this; see the docs - itertag = 'item' # change it accordingly - - def parse_node(self, response, selector): - item = {} - #item['url'] = selector.select('url').get() - #item['name'] = selector.select('name').get() - #item['description'] = selector.select('description').get() - return item diff --git a/tests/test_commands.py b/tests/test_commands.py index c25495d16..f3fe45139 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2,6 +2,7 @@ import inspect import json import optparse import os +from stat import S_IWRITE as ANYONE_WRITE_PERMISSION import subprocess import sys import tempfile @@ -17,7 +18,7 @@ from twisted.trial import unittest import scrapy from scrapy.commands import ScrapyCommand -from scrapy.commands.startproject import IGNORE +from scrapy.commands.startproject import IGNORE, _make_writable from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import get_testenv @@ -170,14 +171,14 @@ class StartprojectTemplatesTest(ProjectTest): template folder has the same permissions as in the project, i.e. everything is writable.""" scrapy_path = scrapy.__path__[0] - templates_dir = os.path.join(scrapy_path, 'templates', 'project') + project_template = os.path.join(scrapy_path, 'templates', 'project') project_name = 'startproject1' renamings = ( ('module', project_name), ('.tmpl', ''), ) expected_permissions = get_permissions_dict( - templates_dir, + project_template, renamings, IGNORE, ) @@ -209,22 +210,30 @@ class StartprojectTemplatesTest(ProjectTest): See https://github.com/scrapy/scrapy/pull/4604 """ scrapy_path = scrapy.__path__[0] - templates_dir = os.path.join(scrapy_path, 'templates', 'project') + templates_dir = os.path.join(scrapy_path, 'templates') + project_template = os.path.join(templates_dir, 'project') project_name = 'startproject2' renamings = ( ('module', project_name), ('.tmpl', ''), ) expected_permissions = get_permissions_dict( - templates_dir, + project_template, renamings, IGNORE, ) - tests_path = os.path.dirname(__file__) - read_only_templates_dir = os.path.join( - tests_path, 'sample_data', 'read_only_templates' - ) + def _make_read_only(path): + current_permissions = os.stat(path).st_mode + os.chmod(path, current_permissions & ~ANYONE_WRITE_PERMISSION) + + read_only_templates_dir = str(Path(mkdtemp()) / 'templates') + copytree(templates_dir, read_only_templates_dir) + + for root, dirs, files in os.walk(read_only_templates_dir): + for node in chain(dirs, files): + _make_read_only(os.path.join(root, node)) + destination = mkdtemp() process = subprocess.Popen( ( @@ -250,14 +259,14 @@ class StartprojectTemplatesTest(ProjectTest): """Check that pre-existing folders and files in the destination folder do not see their permissions modified.""" scrapy_path = scrapy.__path__[0] - templates_dir = os.path.join(scrapy_path, 'templates', 'project') + project_template = os.path.join(scrapy_path, 'templates', 'project') project_name = 'startproject3' renamings = ( ('module', project_name), ('.tmpl', ''), ) expected_permissions = get_permissions_dict( - templates_dir, + project_template, renamings, IGNORE, ) From 7e386157033e69bccc56e3a34d5545daa0174d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Tue, 7 Jul 2020 15:30:19 +0200 Subject: [PATCH 42/56] Remove unused import --- tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index f3fe45139..002237824 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -18,7 +18,7 @@ from twisted.trial import unittest import scrapy from scrapy.commands import ScrapyCommand -from scrapy.commands.startproject import IGNORE, _make_writable +from scrapy.commands.startproject import IGNORE from scrapy.settings import Settings from scrapy.utils.python import to_unicode from scrapy.utils.test import get_testenv From 9e99be982a2916559219693e6665ec7e9319f0e9 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Wed, 17 Jun 2020 15:52:57 -0300 Subject: [PATCH 43/56] Remove backslash --- scrapy/cmdline.py | 10 ++++++---- scrapy/commands/crawl.py | 6 ++++-- scrapy/commands/fetch.py | 6 ++++-- scrapy/commands/genspider.py | 7 ++++--- scrapy/commands/startproject.py | 7 ++++--- scrapy/commands/view.py | 3 +-- scrapy/core/engine.py | 8 +++++--- scrapy/downloadermiddlewares/redirect.py | 13 +++++++------ scrapy/downloadermiddlewares/retry.py | 6 ++++-- scrapy/extensions/memusage.py | 12 ++++++++---- scrapy/http/request/form.py | 3 +-- scrapy/http/response/text.py | 5 ++++- scrapy/link.py | 14 ++++++++++---- scrapy/settings/__init__.py | 3 +-- scrapy/utils/benchserver.py | 3 +-- scrapy/utils/conf.py | 12 +++++++----- scrapy/utils/curl.py | 3 +-- scrapy/utils/ossignal.py | 3 +-- scrapy/utils/response.py | 16 ++++++++++------ scrapy/utils/spider.py | 10 ++++++---- 20 files changed, 89 insertions(+), 61 deletions(-) diff --git a/scrapy/cmdline.py b/scrapy/cmdline.py index b189e016b..3e88536e4 100644 --- a/scrapy/cmdline.py +++ b/scrapy/cmdline.py @@ -19,10 +19,12 @@ def _iter_command_classes(module_name): # scrapy.utils.spider.iter_spider_classes for module in walk_modules(module_name): for obj in vars(module).values(): - if inspect.isclass(obj) and \ - issubclass(obj, ScrapyCommand) and \ - obj.__module__ == module.__name__ and \ - not obj == ScrapyCommand: + if ( + inspect.isclass(obj) + and issubclass(obj, ScrapyCommand) + and obj.__module__ == module.__name__ + and not obj == ScrapyCommand + ): yield obj diff --git a/scrapy/commands/crawl.py b/scrapy/commands/crawl.py index e1724c1e6..f205c40b0 100644 --- a/scrapy/commands/crawl.py +++ b/scrapy/commands/crawl.py @@ -26,6 +26,8 @@ class Command(BaseRunSpiderCommand): else: self.crawler_process.start() - if self.crawler_process.bootstrap_failed or \ - (hasattr(self.crawler_process, 'has_exception') and self.crawler_process.has_exception): + if ( + self.crawler_process.bootstrap_failed + or hasattr(self.crawler_process, 'has_exception') and self.crawler_process.has_exception + ): self.exitcode = 1 diff --git a/scrapy/commands/fetch.py b/scrapy/commands/fetch.py index 063195f50..95f87e8c3 100644 --- a/scrapy/commands/fetch.py +++ b/scrapy/commands/fetch.py @@ -19,8 +19,10 @@ class Command(ScrapyCommand): return "Fetch a URL using the Scrapy downloader" def long_desc(self): - return "Fetch a URL using the Scrapy downloader and print its content " \ - "to stdout. You may want to use --nolog to disable logging" + return ( + "Fetch a URL using the Scrapy downloader and print its content" + " to stdout. You may want to use --nolog to disable logging" + ) def add_options(self, parser): ScrapyCommand.add_options(self, parser) diff --git a/scrapy/commands/genspider.py b/scrapy/commands/genspider.py index abf3b7a5c..4c7548e9c 100644 --- a/scrapy/commands/genspider.py +++ b/scrapy/commands/genspider.py @@ -121,6 +121,7 @@ class Command(ScrapyCommand): @property def templates_dir(self): - _templates_base_dir = self.settings['TEMPLATES_DIR'] or \ - join(scrapy.__path__[0], 'templates') - return join(_templates_base_dir, 'spiders') + return join( + self.settings['TEMPLATES_DIR'] or join(scrapy.__path__[0], 'templates'), + 'spiders' + ) diff --git a/scrapy/commands/startproject.py b/scrapy/commands/startproject.py index 852281959..ae4a15b0f 100644 --- a/scrapy/commands/startproject.py +++ b/scrapy/commands/startproject.py @@ -137,6 +137,7 @@ class Command(ScrapyCommand): @property def templates_dir(self): - _templates_base_dir = self.settings['TEMPLATES_DIR'] or \ - join(scrapy.__path__[0], 'templates') - return join(_templates_base_dir, 'project') + return join( + self.settings['TEMPLATES_DIR'] or join(scrapy.__path__[0], 'templates'), + 'project' + ) diff --git a/scrapy/commands/view.py b/scrapy/commands/view.py index 41e77ba3b..908bee966 100644 --- a/scrapy/commands/view.py +++ b/scrapy/commands/view.py @@ -8,8 +8,7 @@ class Command(fetch.Command): return "Open URL in browser, as seen by Scrapy" def long_desc(self): - return "Fetch a URL using the Scrapy downloader and show its " \ - "contents in a browser" + return "Fetch a URL using the Scrapy downloader and show its contents in a browser" def add_options(self, parser): super(Command, self).add_options(parser) diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index de0da4b70..86a6abb23 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -141,10 +141,12 @@ class ExecutionEngine: def _needs_backout(self, spider): slot = self.slot - return not self.running \ - or slot.closing \ - or self.downloader.needs_backout() \ + return ( + not self.running + or slot.closing + or self.downloader.needs_backout() or self.scraper.slot.needs_backout() + ) def _next_request_from_scheduler(self, spider): slot = self.slot diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index b32afb8e4..366d60dcb 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -33,10 +33,8 @@ class BaseRedirectMiddleware: if ttl and redirects <= self.max_redirect_times: redirected.meta['redirect_times'] = redirects redirected.meta['redirect_ttl'] = ttl - 1 - redirected.meta['redirect_urls'] = request.meta.get('redirect_urls', []) + \ - [request.url] - redirected.meta['redirect_reasons'] = request.meta.get('redirect_reasons', []) + \ - [reason] + redirected.meta['redirect_urls'] = request.meta.get('redirect_urls', []) + [request.url] + redirected.meta['redirect_reasons'] = request.meta.get('redirect_reasons', []) + [reason] redirected.dont_filter = request.dont_filter redirected.priority = request.priority + self.priority_adjust logger.debug("Redirecting (%(reason)s) to %(redirected)s from %(request)s", @@ -99,8 +97,11 @@ class MetaRefreshMiddleware(BaseRedirectMiddleware): self._maxdelay = settings.getint('METAREFRESH_MAXDELAY') def process_response(self, request, response, spider): - if request.meta.get('dont_redirect', False) or request.method == 'HEAD' or \ - not isinstance(response, HtmlResponse): + if ( + request.meta.get('dont_redirect', False) + or request.method == 'HEAD' + or not isinstance(response, HtmlResponse) + ): return response interval, url = get_meta_refresh(response, diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index 6d11af5b2..67be8c282 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -60,8 +60,10 @@ class RetryMiddleware: return response def process_exception(self, request, exception, spider): - if isinstance(exception, self.EXCEPTIONS_TO_RETRY) \ - and not request.meta.get('dont_retry', False): + if ( + isinstance(exception, self.EXCEPTIONS_TO_RETRY) + and not request.meta.get('dont_retry', False) + ): return self._retry(request, exception, spider) def _retry(self, request, reason, spider): diff --git a/scrapy/extensions/memusage.py b/scrapy/extensions/memusage.py index a0540bf8f..ab2e43e8c 100644 --- a/scrapy/extensions/memusage.py +++ b/scrapy/extensions/memusage.py @@ -81,8 +81,10 @@ class MemoryUsage: logger.error("Memory usage exceeded %(memusage)dM. Shutting down Scrapy...", {'memusage': mem}, extra={'crawler': self.crawler}) if self.notify_mails: - subj = "%s terminated: memory usage exceeded %dM at %s" % \ - (self.crawler.settings['BOT_NAME'], mem, socket.gethostname()) + subj = ( + "%s terminated: memory usage exceeded %dM at %s" + % (self.crawler.settings['BOT_NAME'], mem, socket.gethostname()) + ) self._send_report(self.notify_mails, subj) self.crawler.stats.set_value('memusage/limit_notified', 1) @@ -102,8 +104,10 @@ class MemoryUsage: logger.warning("Memory usage reached %(memusage)dM", {'memusage': mem}, extra={'crawler': self.crawler}) if self.notify_mails: - subj = "%s warning: memory usage reached %dM at %s" % \ - (self.crawler.settings['BOT_NAME'], mem, socket.gethostname()) + subj = ( + "%s warning: memory usage reached %dM at %s" + % (self.crawler.settings['BOT_NAME'], mem, socket.gethostname()) + ) self._send_report(self.notify_mails, subj) self.crawler.stats.set_value('memusage/warning_notified', 1) self.warned = True diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py index cd4e3373f..0e6ceef0b 100644 --- a/scrapy/http/request/form.py +++ b/scrapy/http/request/form.py @@ -205,8 +205,7 @@ def _get_clickable(clickdata, form): # We didn't find it, so now we build an XPath expression out of the other # arguments, because they can be used as such - xpath = u'.//*' + \ - u''.join(u'[@%s="%s"]' % c for c in clickdata.items()) + xpath = u'.//*' + u''.join(u'[@%s="%s"]' % c for c in clickdata.items()) el = form.xpath(xpath) if len(el) == 1: return (el[0].get('name'), el[0].get('value') or '') diff --git a/scrapy/http/response/text.py b/scrapy/http/response/text.py index b43fe5c19..0f300c8da 100644 --- a/scrapy/http/response/text.py +++ b/scrapy/http/response/text.py @@ -62,8 +62,11 @@ class TextResponse(Response): return self._declared_encoding() or self._body_inferred_encoding() def _declared_encoding(self): - return self._encoding or self._headers_encoding() \ + return ( + self._encoding + or self._headers_encoding() or self._body_declared_encoding() + ) def body_as_unicode(self): """Return body as unicode""" diff --git a/scrapy/link.py b/scrapy/link.py index 7cb0765cc..1ef50b113 100644 --- a/scrapy/link.py +++ b/scrapy/link.py @@ -21,12 +21,18 @@ class Link: self.nofollow = nofollow def __eq__(self, other): - return self.url == other.url and self.text == other.text and \ - self.fragment == other.fragment and self.nofollow == other.nofollow + return ( + self.url == other.url + and self.text == other.text + and self.fragment == other.fragment + and self.nofollow == other.nofollow + ) def __hash__(self): return hash(self.url) ^ hash(self.text) ^ hash(self.fragment) ^ hash(self.nofollow) def __repr__(self): - return 'Link(url=%r, text=%r, fragment=%r, nofollow=%r)' % \ - (self.url, self.text, self.fragment, self.nofollow) + return ( + 'Link(url=%r, text=%r, fragment=%r, nofollow=%r)' + % (self.url, self.text, self.fragment, self.nofollow) + ) diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index b9a13c018..ff8317cd1 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -52,8 +52,7 @@ class SettingsAttribute: self.priority = priority def __str__(self): - return "<SettingsAttribute value={self.value!r} " \ - "priority={self.priority}>".format(self=self) + return "<SettingsAttribute value={self.value!r} priority={self.priority}>".format(self=self) __repr__ = __str__ diff --git a/scrapy/utils/benchserver.py b/scrapy/utils/benchserver.py index 9d8d64612..f595a1acb 100644 --- a/scrapy/utils/benchserver.py +++ b/scrapy/utils/benchserver.py @@ -28,8 +28,7 @@ class Root(Resource): def _getarg(request, name, default=None, type=str): - return type(request.args[name][0]) \ - if name in request.args else default + return type(request.args[name][0]) if name in request.args else default if __name__ == '__main__': diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py index 5921f82bf..728bb5f1b 100644 --- a/scrapy/utils/conf.py +++ b/scrapy/utils/conf.py @@ -101,11 +101,13 @@ def get_config(use_closest=True): def get_sources(use_closest=True): - xdg_config_home = os.environ.get('XDG_CONFIG_HOME') or \ - os.path.expanduser('~/.config') - sources = ['/etc/scrapy.cfg', r'c:\scrapy\scrapy.cfg', - xdg_config_home + '/scrapy.cfg', - os.path.expanduser('~/.scrapy.cfg')] + xdg_config_home = os.environ.get('XDG_CONFIG_HOME') or os.path.expanduser('~/.config') + sources = [ + '/etc/scrapy.cfg', + r'c:\scrapy\scrapy.cfg', + xdg_config_home + '/scrapy.cfg', + os.path.expanduser('~/.scrapy.cfg'), + ] if use_closest: sources.append(closest_scrapy_cfg()) return sources diff --git a/scrapy/utils/curl.py b/scrapy/utils/curl.py index 67b22dbc5..aa681522f 100644 --- a/scrapy/utils/curl.py +++ b/scrapy/utils/curl.py @@ -9,8 +9,7 @@ from w3lib.http import basic_auth_header class CurlParser(argparse.ArgumentParser): def error(self, message): - error_msg = \ - 'There was an error parsing the curl command: {}'.format(message) + error_msg = 'There was an error parsing the curl command: {}'.format(message) raise ValueError(error_msg) diff --git a/scrapy/utils/ossignal.py b/scrapy/utils/ossignal.py index 45c9cef0c..cf867f3f8 100644 --- a/scrapy/utils/ossignal.py +++ b/scrapy/utils/ossignal.py @@ -18,8 +18,7 @@ def install_shutdown_handlers(function, override_sigint=True): from twisted.internet import reactor reactor._handleSignals() signal.signal(signal.SIGTERM, function) - if signal.getsignal(signal.SIGINT) == signal.default_int_handler or \ - override_sigint: + if signal.getsignal(signal.SIGINT) == signal.default_int_handler or override_sigint: signal.signal(signal.SIGINT, function) # Catch Ctrl-Break in windows if hasattr(signal, 'SIGBREAK'): diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index edbc0db25..c29b619ce 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -47,13 +47,17 @@ def response_httprepr(response): is provided only for reference, since it's not the exact stream of bytes that was received (that's not exposed by Twisted). """ - s = b"HTTP/1.1 " + to_bytes(str(response.status)) + b" " + \ - to_bytes(http.RESPONSES.get(response.status, b'')) + b"\r\n" + values = [ + b"HTTP/1.1 ", + to_bytes(str(response.status)), + b" ", + to_bytes(http.RESPONSES.get(response.status, b'')), + b"\r\n", + ] if response.headers: - s += response.headers.to_string() + b"\r\n" - s += b"\r\n" - s += response.body - return s + values.extend([response.headers.to_string(), b"\r\n"]) + values.extend([b"\r\n", response.body]) + return b"".join(values) def open_in_browser(response, _openfunc=webbrowser.open): diff --git a/scrapy/utils/spider.py b/scrapy/utils/spider.py index 7e7a50c88..f3a9a67a3 100644 --- a/scrapy/utils/spider.py +++ b/scrapy/utils/spider.py @@ -34,10 +34,12 @@ def iter_spider_classes(module): from scrapy.spiders import Spider for obj in vars(module).values(): - if inspect.isclass(obj) and \ - issubclass(obj, Spider) and \ - obj.__module__ == module.__name__ and \ - getattr(obj, 'name', None): + if ( + inspect.isclass(obj) + and issubclass(obj, Spider) + and obj.__module__ == module.__name__ + and getattr(obj, 'name', None) + ): yield obj From 9aea1f096171d38348b1403302c6c40eeef7f0a6 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta <eugenio.lacuesta@gmail.com> Date: Thu, 9 Jul 2020 11:04:46 -0300 Subject: [PATCH 44/56] Remove backslash (tests) --- ...st_downloadermiddleware_httpcompression.py | 3 +- tests/test_downloadermiddleware_redirect.py | 18 ++++------- tests/test_selector.py | 3 +- tests/test_settings/__init__.py | 3 +- tests/test_spider.py | 10 ++++-- tests/test_spidermiddleware_referer.py | 32 +++++++++++++------ tests/test_utils_curl.py | 6 ++-- tests/test_utils_defer.py | 9 ++++-- tests/test_utils_iterators.py | 30 +++++++++-------- tests/test_utils_request.py | 8 +++-- tests/test_utils_response.py | 3 +- tests/test_utils_sitemap.py | 3 +- tests/test_utils_url.py | 3 +- tests/test_webclient.py | 5 ++- 14 files changed, 75 insertions(+), 61 deletions(-) diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 87304d76c..a806f55ce 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -5,8 +5,7 @@ from gzip import GzipFile from scrapy.spiders import Spider from scrapy.http import Response, Request, HtmlResponse -from scrapy.downloadermiddlewares.httpcompression import HttpCompressionMiddleware, \ - ACCEPTED_ENCODINGS +from scrapy.downloadermiddlewares.httpcompression import HttpCompressionMiddleware, ACCEPTED_ENCODINGS from scrapy.responsetypes import responsetypes from scrapy.utils.gz import gunzip from tests import tests_datadir diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index c46b1bb87..919dbed23 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -77,12 +77,9 @@ class RedirectMiddlewareTest(unittest.TestCase): assert isinstance(req2, Request) self.assertEqual(req2.url, url2) self.assertEqual(req2.method, 'GET') - assert 'Content-Type' not in req2.headers, \ - "Content-Type header must not be present in redirected request" - assert 'Content-Length' not in req2.headers, \ - "Content-Length header must not be present in redirected request" - assert not req2.body, \ - "Redirected body must be empty, not '%s'" % req2.body + assert 'Content-Type' not in req2.headers, "Content-Type header must not be present in redirected request" + assert 'Content-Length' not in req2.headers, "Content-Length header must not be present in redirected request" + assert not req2.body, "Redirected body must be empty, not '%s'" % req2.body # response without Location header but with status code is 3XX should be ignored del rsp.headers['Location'] @@ -244,12 +241,9 @@ class MetaRefreshMiddlewareTest(unittest.TestCase): assert isinstance(req2, Request) self.assertEqual(req2.url, 'http://example.org/newpage') self.assertEqual(req2.method, 'GET') - assert 'Content-Type' not in req2.headers, \ - "Content-Type header must not be present in redirected request" - assert 'Content-Length' not in req2.headers, \ - "Content-Length header must not be present in redirected request" - assert not req2.body, \ - "Redirected body must be empty, not '%s'" % req2.body + assert 'Content-Type' not in req2.headers, "Content-Type header must not be present in redirected request" + assert 'Content-Length' not in req2.headers, "Content-Length header must not be present in redirected request" + assert not req2.body, "Redirected body must be empty, not '%s'" % req2.body def test_max_redirect_times(self): self.mw.max_redirect_times = 1 diff --git a/tests/test_selector.py b/tests/test_selector.py index bcf653444..00e663c11 100644 --- a/tests/test_selector.py +++ b/tests/test_selector.py @@ -88,8 +88,7 @@ class SelectorTestCase(unittest.TestCase): """Check that classes are using slots and are weak-referenceable""" x = Selector(text='') weakref.ref(x) - assert not hasattr(x, '__dict__'), "%s does not use __slots__" % \ - x.__class__.__name__ + assert not hasattr(x, '__dict__'), "%s does not use __slots__" % x.__class__.__name__ def test_selector_bad_args(self): with self.assertRaisesRegex(ValueError, 'received both response and text'): diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py index 2da6aa4b5..6e56a28f5 100644 --- a/tests/test_settings/__init__.py +++ b/tests/test_settings/__init__.py @@ -86,8 +86,7 @@ class BaseSettingsTest(unittest.TestCase): def test_set_calls_settings_attributes_methods_on_update(self): attr = SettingsAttribute('value', 10) - with mock.patch.object(attr, '__setattr__') as mock_setattr, \ - mock.patch.object(attr, 'set') as mock_set: + with mock.patch.object(attr, '__setattr__') as mock_setattr, mock.patch.object(attr, 'set') as mock_set: self.settings.attributes = {'TEST_OPTION': attr} diff --git a/tests/test_spider.py b/tests/test_spider.py index 805d70459..bd9238810 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -11,8 +11,14 @@ from scrapy import signals from scrapy.settings import Settings from scrapy.http import Request, Response, TextResponse, XmlResponse, HtmlResponse from scrapy.spiders.init import InitSpider -from scrapy.spiders import Spider, CrawlSpider, Rule, XMLFeedSpider, \ - CSVFeedSpider, SitemapSpider +from scrapy.spiders import ( + CSVFeedSpider, + CrawlSpider, + Rule, + SitemapSpider, + Spider, + XMLFeedSpider, +) from scrapy.linkextractors import LinkExtractor from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.test import get_crawler diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index 067118cf0..5141f47af 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -6,16 +6,28 @@ from scrapy.http import Response, Request from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.downloadermiddlewares.redirect import RedirectMiddleware -from scrapy.spidermiddlewares.referer import RefererMiddleware, \ - POLICY_NO_REFERRER, POLICY_NO_REFERRER_WHEN_DOWNGRADE, \ - POLICY_SAME_ORIGIN, POLICY_ORIGIN, POLICY_ORIGIN_WHEN_CROSS_ORIGIN, \ - POLICY_SCRAPY_DEFAULT, POLICY_UNSAFE_URL, \ - POLICY_STRICT_ORIGIN, POLICY_STRICT_ORIGIN_WHEN_CROSS_ORIGIN, \ - DefaultReferrerPolicy, \ - NoReferrerPolicy, NoReferrerWhenDowngradePolicy, \ - OriginWhenCrossOriginPolicy, OriginPolicy, \ - StrictOriginWhenCrossOriginPolicy, StrictOriginPolicy, \ - SameOriginPolicy, UnsafeUrlPolicy, ReferrerPolicy +from scrapy.spidermiddlewares.referer import ( + DefaultReferrerPolicy, + NoReferrerPolicy, + NoReferrerWhenDowngradePolicy, + OriginPolicy, + OriginWhenCrossOriginPolicy, + POLICY_NO_REFERRER, + POLICY_NO_REFERRER_WHEN_DOWNGRADE, + POLICY_ORIGIN, + POLICY_ORIGIN_WHEN_CROSS_ORIGIN, + POLICY_SAME_ORIGIN, + POLICY_SCRAPY_DEFAULT, + POLICY_STRICT_ORIGIN, + POLICY_STRICT_ORIGIN_WHEN_CROSS_ORIGIN, + POLICY_UNSAFE_URL, + RefererMiddleware, + ReferrerPolicy, + SameOriginPolicy, + StrictOriginPolicy, + StrictOriginWhenCrossOriginPolicy, + UnsafeUrlPolicy, +) class TestRefererMiddleware(TestCase): diff --git a/tests/test_utils_curl.py b/tests/test_utils_curl.py index 299a51efe..6b05c8771 100644 --- a/tests/test_utils_curl.py +++ b/tests/test_utils_curl.py @@ -29,8 +29,7 @@ class CurlToRequestKwargsTest(unittest.TestCase): self._test_command(curl_command, expected_result) def test_get_basic_auth(self): - curl_command = 'curl "https://api.test.com/" -u ' \ - '"some_username:some_password"' + curl_command = 'curl "https://api.test.com/" -u "some_username:some_password"' expected_result = { "method": "GET", "url": "https://api.test.com/", @@ -212,8 +211,7 @@ class CurlToRequestKwargsTest(unittest.TestCase): with warnings.catch_warnings(): # avoid warning when executing tests warnings.simplefilter('ignore') curl_command = 'curl --bar --baz http://www.example.com' - expected_result = \ - {"method": "GET", "url": "http://www.example.com"} + expected_result = {"method": "GET", "url": "http://www.example.com"} self.assertEqual(curl_to_request_kwargs(curl_command), expected_result) # case 2: ignore_unknown_options=False (raise exception): diff --git a/tests/test_utils_defer.py b/tests/test_utils_defer.py index 2d4b88121..8c84331b9 100644 --- a/tests/test_utils_defer.py +++ b/tests/test_utils_defer.py @@ -2,8 +2,13 @@ from twisted.trial import unittest from twisted.internet import reactor, defer from twisted.python.failure import Failure -from scrapy.utils.defer import mustbe_deferred, process_chain, \ - process_chain_both, process_parallel, iter_errback +from scrapy.utils.defer import ( + iter_errback, + mustbe_deferred, + process_chain, + process_chain_both, + process_parallel, +) class MustbeDeferredTest(unittest.TestCase): diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index 8344c6701..3ebe3ac24 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -15,18 +15,20 @@ class XmliterTestCase(unittest.TestCase): xmliter = staticmethod(xmliter) def test_xmliter(self): - body = b"""<?xml version="1.0" encoding="UTF-8"?>\ + body = b""" + <?xml version="1.0" encoding="UTF-8"?> <products xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xsi:noNamespaceSchemaLocation="someschmea.xsd">\ - <product id="001">\ - <type>Type 1</type>\ - <name>Name 1</name>\ - </product>\ - <product id="002">\ - <type>Type 2</type>\ - <name>Name 2</name>\ - </product>\ - </products>""" + xsi:noNamespaceSchemaLocation="someschmea.xsd"> + <product id="001"> + <type>Type 1</type> + <name>Name 1</name> + </product> + <product id="002"> + <type>Type 2</type> + <name>Name 2</name> + </product> + </products> + """ response = XmlResponse(url="http://example.com", body=body) attrs = [] @@ -115,7 +117,7 @@ class XmliterTestCase(unittest.TestCase): [[u'one'], [u'two']]) def test_xmliter_namespaces(self): - body = b"""\ + body = b""" <?xml version="1.0" encoding="UTF-8"?> <rss version="2.0" xmlns:g="http://base.google.com/ns/1.0"> <channel> @@ -185,7 +187,7 @@ class LxmlXmliterTestCase(XmliterTestCase): xmliter = staticmethod(xmliter_lxml) def test_xmliter_iterate_namespace(self): - body = b"""\ + body = b""" <?xml version="1.0" encoding="UTF-8"?> <rss version="2.0" xmlns="http://base.google.com/ns/1.0"> <channel> @@ -214,7 +216,7 @@ class LxmlXmliterTestCase(XmliterTestCase): self.assertEqual(node.xpath('text()').getall(), ['http://www.mydummycompany.com/images/item2.jpg']) def test_xmliter_namespaces_prefix(self): - body = b"""\ + body = b""" <?xml version="1.0" encoding="UTF-8"?> <root> <h:table xmlns:h="http://www.w3.org/TR/html4/"> diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index 4cd4b7010..7e0049b1d 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -1,7 +1,11 @@ import unittest from scrapy.http import Request -from scrapy.utils.request import request_fingerprint, _fingerprint_cache, \ - request_authenticate, request_httprepr +from scrapy.utils.request import ( + _fingerprint_cache, + request_authenticate, + request_fingerprint, + request_httprepr, +) class UtilsRequestTest(unittest.TestCase): diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 6ebf290c0..d6f4c0bb5 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -37,8 +37,7 @@ class ResponseUtilsTest(unittest.TestCase): self.assertIn(b'<base href="' + to_bytes(url) + b'">', bbody) return True response = HtmlResponse(url, body=body) - assert open_in_browser(response, _openfunc=browser_open), \ - "Browser not called" + assert open_in_browser(response, _openfunc=browser_open), "Browser not called" resp = Response(url, body=body) self.assertRaises(TypeError, open_in_browser, resp, debug=True) diff --git a/tests/test_utils_sitemap.py b/tests/test_utils_sitemap.py index bfbf9abb3..23eb261b7 100644 --- a/tests/test_utils_sitemap.py +++ b/tests/test_utils_sitemap.py @@ -156,8 +156,7 @@ Disallow: /forum/active/ def test_sitemap_blanklines(self): """Assert we can deal with starting blank lines before <xml> tag""" - s = Sitemap(b"""\ - + s = Sitemap(b""" <?xml version="1.0" encoding="UTF-8"?> <sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"> diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 09a6d6c70..a194a0998 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -207,8 +207,7 @@ def create_guess_scheme_t(args): def do_expected(self): url = guess_scheme(args[0]) assert url.startswith(args[1]), \ - 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( - args[0], url, args[1]) + 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % (args[0], url, args[1]) return do_expected diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 188e54602..c1c5945c2 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -356,9 +356,8 @@ class WebClientTestCase(unittest.TestCase): """ Test that non-standart body encoding matches Content-Encoding header """ body = b'\xd0\x81\xd1\x8e\xd0\xaf' - return getPage( - self.getURL('encoding'), body=body, response_transform=lambda r: r)\ - .addCallback(self._check_Encoding, body) + dfd = getPage(self.getURL('encoding'), body=body, response_transform=lambda r: r) + return dfd.addCallback(self._check_Encoding, body) def _check_Encoding(self, response, original_body): content_encoding = to_unicode(response.headers[b'Content-Encoding']) From a6a5fa91da8944943e2c9d8f34f09662be17b781 Mon Sep 17 00:00:00 2001 From: Artur Shellunts <shellunts.artur@gmail.com> Date: Fri, 10 Jul 2020 23:10:49 +0200 Subject: [PATCH 45/56] Remove deprecated class HtmlParserLinkExtractor Issue #4356 --- scrapy/linkextractors/htmlparser.py | 91 ----------------------------- 1 file changed, 91 deletions(-) delete mode 100644 scrapy/linkextractors/htmlparser.py diff --git a/scrapy/linkextractors/htmlparser.py b/scrapy/linkextractors/htmlparser.py deleted file mode 100644 index 0425d4340..000000000 --- a/scrapy/linkextractors/htmlparser.py +++ /dev/null @@ -1,91 +0,0 @@ -""" -HTMLParser-based link extractor -""" -import warnings -from html.parser import HTMLParser -from urllib.parse import urljoin - -from w3lib.url import safe_url_string -from w3lib.html import strip_html5_whitespace - -from scrapy.link import Link -from scrapy.utils.python import unique as unique_list -from scrapy.exceptions import ScrapyDeprecationWarning - - -class HtmlParserLinkExtractor(HTMLParser): - - def __init__(self, tag="a", attr="href", process=None, unique=False, - strip=True): - HTMLParser.__init__(self) - - warnings.warn( - "HtmlParserLinkExtractor is deprecated and will be removed in " - "future releases. Please use scrapy.linkextractors.LinkExtractor", - ScrapyDeprecationWarning, stacklevel=2, - ) - - self.scan_tag = tag if callable(tag) else lambda t: t == tag - self.scan_attr = attr if callable(attr) else lambda a: a == attr - self.process_attr = process if callable(process) else lambda v: v - self.unique = unique - self.strip = strip - - def _extract_links(self, response_text, response_url, response_encoding): - self.reset() - self.feed(response_text) - self.close() - - links = unique_list(self.links, key=lambda link: link.url) if self.unique else self.links - - ret = [] - base_url = urljoin(response_url, self.base_url) if self.base_url else response_url - for link in links: - if isinstance(link.url, str): - link.url = link.url.encode(response_encoding) - try: - link.url = urljoin(base_url, link.url) - except ValueError: - continue - link.url = safe_url_string(link.url, response_encoding) - link.text = link.text.decode(response_encoding) - ret.append(link) - - return ret - - def extract_links(self, response): - # wrapper needed to allow to work directly with text - return self._extract_links(response.body, response.url, response.encoding) - - def reset(self): - HTMLParser.reset(self) - - self.base_url = None - self.current_link = None - self.links = [] - - def handle_starttag(self, tag, attrs): - if tag == 'base': - self.base_url = dict(attrs).get('href') - if self.scan_tag(tag): - for attr, value in attrs: - if self.scan_attr(attr): - if self.strip: - value = strip_html5_whitespace(value) - url = self.process_attr(value) - link = Link(url=url) - self.links.append(link) - self.current_link = link - - def handle_endtag(self, tag): - if self.scan_tag(tag): - self.current_link = None - - def handle_data(self, data): - if self.current_link: - self.current_link.text = self.current_link.text + data - - def matches(self, url): - """This extractor matches with any url, since - it doesn't contain any patterns""" - return True From 3f7e8635f479f4de2ab1c3d518010730a5f6f0a6 Mon Sep 17 00:00:00 2001 From: Aditya Kumar <k.aditya00@gmail.com> Date: Sat, 11 Jul 2020 12:18:24 +0530 Subject: [PATCH 46/56] Allow the parse command to write data to a file (#4377) --- docs/topics/commands.rst | 4 +++- scrapy/commands/__init__.py | 2 +- scrapy/commands/parse.py | 24 ++++++++---------------- tests/test_command_parse.py | 23 ++++++++++++++++++++++- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index a0dcba90d..4fce51abc 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -468,7 +468,7 @@ Supported options: * ``--callback`` or ``-c``: spider method to use as callback for parsing the response -* ``--meta`` or ``-m``: additional request meta that will be passed to the callback +* ``--meta`` or ``-m``: additional request meta that will be passed to the callback request. This must be a valid json string. Example: --meta='{"foo" : "bar"}' * ``--cbkwargs``: additional keyword arguments that will be passed to the callback. @@ -491,6 +491,8 @@ Supported options: * ``--verbose`` or ``-v``: display information for each depth level +* ``--output`` or ``-o``: dump scraped items to a file + .. skip: start Usage example:: diff --git a/scrapy/commands/__init__.py b/scrapy/commands/__init__.py index ab850dcb3..57ce4e522 100644 --- a/scrapy/commands/__init__.py +++ b/scrapy/commands/__init__.py @@ -108,7 +108,7 @@ class ScrapyCommand: class BaseRunSpiderCommand(ScrapyCommand): """ - Common class used to share functionality between the crawl and runspider commands + Common class used to share functionality between the crawl, parse and runspider commands """ def add_options(self, parser): ScrapyCommand.add_options(self, parser) diff --git a/scrapy/commands/parse.py b/scrapy/commands/parse.py index 8b7fa8b58..abc8ba9ff 100644 --- a/scrapy/commands/parse.py +++ b/scrapy/commands/parse.py @@ -4,18 +4,16 @@ import logging from itemadapter import is_item, ItemAdapter from w3lib.url import is_url -from scrapy.commands import ScrapyCommand +from scrapy.commands import BaseRunSpiderCommand from scrapy.http import Request from scrapy.utils import display -from scrapy.utils.conf import arglist_to_dict from scrapy.utils.spider import iterate_spider_output, spidercls_for_request from scrapy.exceptions import UsageError logger = logging.getLogger(__name__) -class Command(ScrapyCommand): - +class Command(BaseRunSpiderCommand): requires_project = True spider = None @@ -31,11 +29,9 @@ class Command(ScrapyCommand): return "Parse URL (using its spider) and print the results" def add_options(self, parser): - ScrapyCommand.add_options(self, parser) + BaseRunSpiderCommand.add_options(self, parser) parser.add_option("--spider", dest="spider", default=None, help="use this spider without looking for one") - parser.add_option("-a", dest="spargs", action="append", default=[], metavar="NAME=VALUE", - help="set spider argument (may be repeated)") parser.add_option("--pipelines", action="store_true", help="process items through pipelines") parser.add_option("--nolinks", dest="nolinks", action="store_true", @@ -200,12 +196,15 @@ class Command(ScrapyCommand): self.add_items(depth, items) self.add_requests(depth, requests) + scraped_data = items if opts.output else [] if depth < opts.depth: for req in requests: req.meta['_depth'] = depth + 1 req.meta['_callback'] = req.callback req.callback = callback - return requests + scraped_data += requests + + return scraped_data # update request meta if any extra meta was passed through the --meta/-m opts. if opts.meta: @@ -221,18 +220,11 @@ class Command(ScrapyCommand): return request def process_options(self, args, opts): - ScrapyCommand.process_options(self, args, opts) + BaseRunSpiderCommand.process_options(self, args, opts) - self.process_spider_arguments(opts) self.process_request_meta(opts) self.process_request_cb_kwargs(opts) - def process_spider_arguments(self, opts): - try: - opts.spargs = arglist_to_dict(opts.spargs) - except ValueError: - raise UsageError("Invalid -a value, use -a NAME=VALUE", print_help=False) - def process_request_meta(self, opts): if opts.meta: try: diff --git a/tests/test_command_parse.py b/tests/test_command_parse.py index a09dcf072..5754a5478 100644 --- a/tests/test_command_parse.py +++ b/tests/test_command_parse.py @@ -1,5 +1,5 @@ import os -from os.path import join, abspath +from os.path import join, abspath, isfile, exists from twisted.internet import defer from scrapy.utils.testsite import SiteTest from scrapy.utils.testproc import ProcessTest @@ -218,3 +218,24 @@ ITEM_PIPELINES = {'%s.pipelines.MyPipeline': 1} ) self.assertRegex(_textmode(out), r"""# Scraped Items -+\n\[\]""") self.assertIn("""Cannot find a rule that matches""", _textmode(stderr)) + + @defer.inlineCallbacks + def test_output_flag(self): + """Checks if a file was created successfully having + correct format containing correct data in it. + """ + file_name = 'data.json' + file_path = join(self.proj_path, file_name) + yield self.execute([ + '--spider', self.spider_name, + '-c', 'parse', + '-o', file_name, + self.url('/html') + ]) + + self.assertTrue(exists(file_path)) + self.assertTrue(isfile(file_path)) + + content = '[\n{},\n{"foo": "bar"}\n]' + with open(file_path, 'r') as f: + self.assertEqual(f.read(), content) From d54c4496ee57785f3d6f882e2d128bb64b6b262c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Mon, 13 Jul 2020 14:36:33 +0200 Subject: [PATCH 47/56] Refactor guess_scheme --- scrapy/utils/url.py | 60 ++++++++++++++++++++++++++++++----------- tests/test_utils_url.py | 33 +++++++++++++++++++++-- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 60e2b50eb..1e431d3bf 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -83,26 +83,54 @@ def add_http_if_no_scheme(url): return url +def _is_posix_path(string): + return bool( + re.match( + r''' + ^ # start with... + ( + \. # ...a single dot, + ( + \. | [^/\.]+ # optionally followed by + )? # either a second dot or some characters + | + ~ # $HOME + )? # optional match of ".", ".." or ".blabla" + / # at least one "/" for a file path, + . # and something after the "/" + ''', + string, + flags=re.VERBOSE, + ) + ) + + +def _is_windows_path(string): + return bool( + re.match( + r''' + ^ + ( + [a-z]:\\ + | \\\\ + ) + ''', + string, + flags=re.IGNORECASE | re.VERBOSE, + ) + ) + + +def _is_path(string): + return _is_posix_path(string) or _is_windows_path(string) + + def guess_scheme(url): """Add an URL scheme if missing: file:// for filepath-like input or http:// otherwise.""" - # POSIX path - if re.match(r'''^ # start with... - ( - \. # ...a single dot, - ( - \. | [^/\.]+ # optionally followed by - )? # either a second dot or some characters - )? # optional match of ".", ".." or ".blabla" - / # at least one "/" for a file path, - . # and something after the "/" - ''', url, flags=re.VERBOSE): + if _is_path(url): return any_to_uri(url) - # Windows drive-letter path - elif re.match(r'''^[a-z]:\\''', url, flags=re.IGNORECASE): - return any_to_uri(url) - else: - return add_http_if_no_scheme(url) + return add_http_if_no_scheme(url) def strip_url(url, strip_credentials=True, strip_default_port=True, origin_only=False, strip_fragment=True): diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 09a6d6c70..6a5254d54 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -1,8 +1,14 @@ import unittest from scrapy.spiders import Spider -from scrapy.utils.url import (url_is_from_any_domain, url_is_from_spider, - add_http_if_no_scheme, guess_scheme, strip_url) +from scrapy.utils.url import ( + add_http_if_no_scheme, + guess_scheme, + _is_path, + strip_url, + url_is_from_any_domain, + url_is_from_spider, +) __doctests__ = ['scrapy.utils.url'] @@ -434,5 +440,28 @@ class StripUrl(unittest.TestCase): self.assertEqual(strip_url(i, origin_only=True), o) +class IsPathTestCase(unittest.TestCase): + + def test_path(self): + for input_value, output_value in ( + # https://en.wikipedia.org/wiki/Path_(computing)#Representations_of_paths_by_operating_system_and_shell + # Unix-like OS, Microsoft Windows / cmd.exe + ("/home/user/docs/Letter.txt", True), + ("./inthisdir", True), + ("../../greatgrandparent", True), + ("~/.rcinfo", True), + (r"C:\user\docs\Letter.txt", True), + ("/user/docs/Letter.txt", True), + (r"C:\Letter.txt", True), + (r"\\Server01\user\docs\Letter.txt", True), + (r"\\?\UNC\Server01\user\docs\Letter.txt", True), + (r"\\?\C:\user\docs\Letter.txt", True), + (r"C:\user\docs\somefile.ext:alternate_stream_name", True), + + (r"https://example.com", False), + ): + self.assertEqual(_is_path(input_value), output_value, input_value) + + if __name__ == "__main__": unittest.main() From 53c323b19d81784e6c376ce8b9602de24d8e3037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Mon, 13 Jul 2020 15:29:30 +0200 Subject: [PATCH 48/56] =?UTF-8?q?=5Fis=5Fpath=20=E2=86=92=20=5Fis=5Ffilesy?= =?UTF-8?q?stem=5Fpath?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/utils/url.py | 4 ++-- tests/test_utils_url.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 1e431d3bf..b23ddb459 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -121,14 +121,14 @@ def _is_windows_path(string): ) -def _is_path(string): +def _is_filesystem_path(string): return _is_posix_path(string) or _is_windows_path(string) def guess_scheme(url): """Add an URL scheme if missing: file:// for filepath-like input or http:// otherwise.""" - if _is_path(url): + if _is_filesystem_path(url): return any_to_uri(url) return add_http_if_no_scheme(url) diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 6a5254d54..3a143ba2f 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -4,7 +4,7 @@ from scrapy.spiders import Spider from scrapy.utils.url import ( add_http_if_no_scheme, guess_scheme, - _is_path, + _is_filesystem_path, strip_url, url_is_from_any_domain, url_is_from_spider, @@ -460,7 +460,7 @@ class IsPathTestCase(unittest.TestCase): (r"https://example.com", False), ): - self.assertEqual(_is_path(input_value), output_value, input_value) + self.assertEqual(_is_filesystem_path(input_value), output_value, input_value) if __name__ == "__main__": From ed5247ca4cbef98d1acf499c888ded74e444ef48 Mon Sep 17 00:00:00 2001 From: Artur Shellunts <shellunts.artur@gmail.com> Date: Tue, 14 Jul 2020 18:06:11 +0200 Subject: [PATCH 49/56] Remove htmlparser.py from tests/ignore.txt --- tests/ignores.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/ignores.txt b/tests/ignores.txt index 45cf6fb92..f6e0d6fbe 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -1,6 +1,5 @@ scrapy/linkextractors/sgml.py scrapy/linkextractors/regex.py -scrapy/linkextractors/htmlparser.py scrapy/downloadermiddlewares/cookies.py scrapy/extensions/statsmailer.py scrapy/extensions/memusage.py From 38496a00b7d2bcfa9d435551409e2c44007d168d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlio=20C=C3=A9sar=20Batista?= <jcb.1611@gmail.com> Date: Wed, 15 Jul 2020 07:08:36 -0300 Subject: [PATCH 50/56] Use the itemlaoders library (#4516) --- docs/conf.py | 13 + docs/topics/loaders.rst | 418 +----------------- scrapy/loader/__init__.py | 269 +++--------- scrapy/loader/common.py | 19 +- scrapy/loader/processors.py | 99 +---- scrapy/utils/misc.py | 6 + setup.cfg | 3 + setup.py | 1 + tests/requirements-py3.txt | 1 - tests/test_loader.py | 531 +---------------------- tests/test_loader_deprecated.py | 720 ++++++++++++++++++++++++++++++++ 11 files changed, 854 insertions(+), 1226 deletions(-) create mode 100644 tests/test_loader_deprecated.py diff --git a/docs/conf.py b/docs/conf.py index 86734fae7..427c79481 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -284,6 +284,7 @@ intersphinx_mapping = { 'attrs': ('https://www.attrs.org/en/stable/', None), 'coverage': ('https://coverage.readthedocs.io/en/stable', None), 'cssselect': ('https://cssselect.readthedocs.io/en/latest', None), + 'itemloaders': ('https://itemloaders.readthedocs.io/en/latest/', None), 'pytest': ('https://docs.pytest.org/en/latest', None), 'python': ('https://docs.python.org/3', None), 'sphinx': ('https://www.sphinx-doc.org/en/master', None), @@ -305,3 +306,15 @@ hoverxref_role_types = { "ref": "tooltip", } hoverxref_roles = ['command', 'reqmeta', 'setting', 'signal'] + + +def setup(app): + app.connect('autodoc-skip-member', maybe_skip_member) + + +def maybe_skip_member(app, what, name, obj, skip, options): + if not skip: + # autodocs was generating a text "alias of" for the following members + # https://github.com/sphinx-doc/sphinx/issues/4422 + return name in {'default_item_class', 'default_selector_class'} + return skip diff --git a/docs/topics/loaders.rst b/docs/topics/loaders.rst index 9c82bb4d9..d0eeb4097 100644 --- a/docs/topics/loaders.rst +++ b/docs/topics/loaders.rst @@ -20,6 +20,10 @@ Item Loaders are designed to provide a flexible, efficient and easy mechanism for extending and overriding different field parsing rules, either by spider, or by source format (HTML, XML, etc) without becoming a nightmare to maintain. +.. note:: Item Loaders are an extension of the itemloaders_ library that make it + easier to work with Scrapy by adding support for + :ref:`responses <topics-request-response>`. + Using Item Loaders to populate items ==================================== @@ -173,8 +177,8 @@ The other thing you need to keep in mind is that the values returned by input processors are collected internally (in lists) and then passed to output processors to populate the fields. -Last, but not least, Scrapy comes with some :ref:`commonly used processors -<topics-loaders-available-processors>` built-in for convenience. +Last, but not least, itemloaders_ comes with some :ref:`commonly used +processors <itemloaders:built-in-processors>` built-in for convenience. Declaring Item Loaders @@ -182,8 +186,8 @@ Declaring Item Loaders Item Loaders are declared using a class definition syntax. Here is an example:: + from itemloaders.processors import TakeFirst, MapCompose, Join from scrapy.loader import ItemLoader - from scrapy.loader.processors import TakeFirst, MapCompose, Join class ProductLoader(ItemLoader): @@ -214,7 +218,7 @@ output processors to use: in the :ref:`Item Field <topics-items-fields>` metadata. Here is an example:: import scrapy - from scrapy.loader.processors import Join, MapCompose, TakeFirst + from itemloaders.processors import Join, MapCompose, TakeFirst from w3lib.html import remove_tags def filter_price(value): @@ -295,250 +299,9 @@ There are several ways to modify Item Loader context values: ItemLoader objects ================== -.. class:: ItemLoader([item, selector, response], **kwargs) - - Return a new Item Loader for populating the given :ref:`item object - <topics-items>`. If no item object is given, one is instantiated - automatically using the class in :attr:`default_item_class`. - - When instantiated with a ``selector`` or a ``response`` parameters - the :class:`ItemLoader` class provides convenient mechanisms for extracting - data from web pages using :ref:`selectors <topics-selectors>`. - - :param item: The item instance to populate using subsequent calls to - :meth:`~ItemLoader.add_xpath`, :meth:`~ItemLoader.add_css`, - or :meth:`~ItemLoader.add_value`. - :type item: :ref:`item object <topics-items>` - - :param selector: The selector to extract data from, when using the - :meth:`add_xpath` (resp. :meth:`add_css`) or :meth:`replace_xpath` - (resp. :meth:`replace_css`) method. - :type selector: :class:`~scrapy.selector.Selector` object - - :param response: The response used to construct the selector using the - :attr:`default_selector_class`, unless the selector argument is given, - in which case this argument is ignored. - :type response: :class:`~scrapy.http.Response` object - - The item, selector, response and the remaining keyword arguments are - assigned to the Loader context (accessible through the :attr:`context` attribute). - - :class:`ItemLoader` instances have the following methods: - - .. method:: get_value(value, *processors, **kwargs) - - Process the given ``value`` by the given ``processors`` and keyword - arguments. - - Available keyword arguments: - - :param re: a regular expression to use for extracting data from the - given value using :meth:`~scrapy.utils.misc.extract_regex` method, - applied before processors - :type re: str or compiled regex - - Examples: - - >>> from scrapy.loader.processors import TakeFirst - >>> loader.get_value(u'name: foo', TakeFirst(), unicode.upper, re='name: (.+)') - 'FOO` - - .. method:: add_value(field_name, value, *processors, **kwargs) - - Process and then add the given ``value`` for the given field. - - The value is first passed through :meth:`get_value` by giving the - ``processors`` and ``kwargs``, and then passed through the - :ref:`field input processor <topics-loaders-processors>` and its result - appended to the data collected for that field. If the field already - contains collected data, the new data is added. - - The given ``field_name`` can be ``None``, in which case values for - multiple fields may be added. And the processed value should be a dict - with field_name mapped to values. - - Examples:: - - loader.add_value('name', u'Color TV') - loader.add_value('colours', [u'white', u'blue']) - loader.add_value('length', u'100') - loader.add_value('name', u'name: foo', TakeFirst(), re='name: (.+)') - loader.add_value(None, {'name': u'foo', 'sex': u'male'}) - - .. method:: replace_value(field_name, value, *processors, **kwargs) - - Similar to :meth:`add_value` but replaces the collected data with the - new value instead of adding it. - .. method:: get_xpath(xpath, *processors, **kwargs) - - Similar to :meth:`ItemLoader.get_value` but receives an XPath instead of a - value, which is used to extract a list of unicode strings from the - selector associated with this :class:`ItemLoader`. - - :param xpath: the XPath to extract data from - :type xpath: str - - :param re: a regular expression to use for extracting data from the - selected XPath region - :type re: str or compiled regex - - Examples:: - - # HTML snippet: <p class="product-name">Color TV</p> - loader.get_xpath('//p[@class="product-name"]') - # HTML snippet: <p id="price">the price is $1200</p> - loader.get_xpath('//p[@id="price"]', TakeFirst(), re='the price is (.*)') - - .. method:: add_xpath(field_name, xpath, *processors, **kwargs) - - Similar to :meth:`ItemLoader.add_value` but receives an XPath instead of a - value, which is used to extract a list of unicode strings from the - selector associated with this :class:`ItemLoader`. - - See :meth:`get_xpath` for ``kwargs``. - - :param xpath: the XPath to extract data from - :type xpath: str - - Examples:: - - # HTML snippet: <p class="product-name">Color TV</p> - loader.add_xpath('name', '//p[@class="product-name"]') - # HTML snippet: <p id="price">the price is $1200</p> - loader.add_xpath('price', '//p[@id="price"]', re='the price is (.*)') - - .. method:: replace_xpath(field_name, xpath, *processors, **kwargs) - - Similar to :meth:`add_xpath` but replaces collected data instead of - adding it. - - .. method:: get_css(css, *processors, **kwargs) - - Similar to :meth:`ItemLoader.get_value` but receives a CSS selector - instead of a value, which is used to extract a list of unicode strings - from the selector associated with this :class:`ItemLoader`. - - :param css: the CSS selector to extract data from - :type css: str - - :param re: a regular expression to use for extracting data from the - selected CSS region - :type re: str or compiled regex - - Examples:: - - # HTML snippet: <p class="product-name">Color TV</p> - loader.get_css('p.product-name') - # HTML snippet: <p id="price">the price is $1200</p> - loader.get_css('p#price', TakeFirst(), re='the price is (.*)') - - .. method:: add_css(field_name, css, *processors, **kwargs) - - Similar to :meth:`ItemLoader.add_value` but receives a CSS selector - instead of a value, which is used to extract a list of unicode strings - from the selector associated with this :class:`ItemLoader`. - - See :meth:`get_css` for ``kwargs``. - - :param css: the CSS selector to extract data from - :type css: str - - Examples:: - - # HTML snippet: <p class="product-name">Color TV</p> - loader.add_css('name', 'p.product-name') - # HTML snippet: <p id="price">the price is $1200</p> - loader.add_css('price', 'p#price', re='the price is (.*)') - - .. method:: replace_css(field_name, css, *processors, **kwargs) - - Similar to :meth:`add_css` but replaces collected data instead of - adding it. - - .. method:: load_item() - - Populate the item with the data collected so far, and return it. The - data collected is first passed through the :ref:`output processors - <topics-loaders-processors>` to get the final value to assign to each - item field. - - .. method:: nested_xpath(xpath) - - Create a nested loader with an xpath selector. - The supplied selector is applied relative to selector associated - with this :class:`ItemLoader`. The nested loader shares the :ref:`item - object <topics-items>` with the parent :class:`ItemLoader` so calls to - :meth:`add_xpath`, :meth:`add_value`, :meth:`replace_value`, etc. will - behave as expected. - - .. method:: nested_css(css) - - Create a nested loader with a css selector. - The supplied selector is applied relative to selector associated - with this :class:`ItemLoader`. The nested loader shares the :ref:`item - object <topics-items>` with the parent :class:`ItemLoader` so calls to - :meth:`add_xpath`, :meth:`add_value`, :meth:`replace_value`, etc. will - behave as expected. - - .. method:: get_collected_values(field_name) - - Return the collected values for the given field. - - .. method:: get_output_value(field_name) - - Return the collected values parsed using the output processor, for the - given field. This method doesn't populate or modify the item at all. - - .. method:: get_input_processor(field_name) - - Return the input processor for the given field. - - .. method:: get_output_processor(field_name) - - Return the output processor for the given field. - - :class:`ItemLoader` instances have the following attributes: - - .. attribute:: item - - The :ref:`item object <topics-items>` being parsed by this Item Loader. - This is mostly used as a property so when attempting to override this - value, you may want to check out :attr:`default_item_class` first. - - .. attribute:: context - - The currently active :ref:`Context <topics-loaders-context>` of this - Item Loader. - - .. attribute:: default_item_class - - An :ref:`item object <topics-items>` class or factory, used to - instantiate items when not given in the ``__init__`` method. - - .. attribute:: default_input_processor - - The default input processor to use for those fields which don't specify - one. - - .. attribute:: default_output_processor - - The default output processor to use for those fields which don't specify - one. - - .. attribute:: default_selector_class - - The class used to construct the :attr:`selector` of this - :class:`ItemLoader`, if only a response is given in the ``__init__`` method. - If a selector is given in the ``__init__`` method this attribute is ignored. - This attribute is sometimes overridden in subclasses. - - .. attribute:: selector - - The :class:`~scrapy.selector.Selector` object to extract data from. - It's either the selector given in the ``__init__`` method or one created from - the response given in the ``__init__`` method using the - :attr:`default_selector_class`. This attribute is meant to be - read-only. +.. autoclass:: scrapy.loader.ItemLoader + :members: + :inherited-members: .. _topics-loaders-nested: @@ -609,7 +372,7 @@ those dashes in the final product names. Here's how you can remove those dashes by reusing and extending the default Product Item Loader (``ProductLoader``):: - from scrapy.loader.processors import MapCompose + from itemloaders.processors import MapCompose from myproject.ItemLoaders import ProductLoader def strip_dashes(x): @@ -622,7 +385,7 @@ Another case where extending Item Loaders can be very helpful is when you have multiple source formats, for example XML and HTML. In the XML version you may want to remove ``CDATA`` occurrences. Here's an example of how to do it:: - from scrapy.loader.processors import MapCompose + from itemloaders.processors import MapCompose from myproject.ItemLoaders import ProductLoader from myproject.utils.xml import remove_cdata @@ -642,156 +405,5 @@ projects. Scrapy only provides the mechanism; it doesn't impose any specific organization of your Loaders collection - that's up to you and your project's needs. -.. _topics-loaders-available-processors: - -Available built-in processors -============================= - -.. module:: scrapy.loader.processors - :synopsis: A collection of processors to use with Item Loaders - -Even though you can use any callable function as input and output processors, -Scrapy provides some commonly used processors, which are described below. Some -of them, like the :class:`MapCompose` (which is typically used as input -processor) compose the output of several functions executed in order, to -produce the final parsed value. - -Here is a list of all built-in processors: - -.. class:: Identity - - The simplest processor, which doesn't do anything. It returns the original - values unchanged. It doesn't receive any ``__init__`` method arguments, nor does it - accept Loader contexts. - - Example: - - >>> from scrapy.loader.processors import Identity - >>> proc = Identity() - >>> proc(['one', 'two', 'three']) - ['one', 'two', 'three'] - -.. class:: TakeFirst - - Returns the first non-null/non-empty value from the values received, - so it's typically used as an output processor to single-valued fields. - It doesn't receive any ``__init__`` method arguments, nor does it accept Loader contexts. - - Example: - - >>> from scrapy.loader.processors import TakeFirst - >>> proc = TakeFirst() - >>> proc(['', 'one', 'two', 'three']) - 'one' - -.. class:: Join(separator=u' ') - - Returns the values joined with the separator given in the ``__init__`` method, which - defaults to ``u' '``. It doesn't accept Loader contexts. - - When using the default separator, this processor is equivalent to the - function: ``u' '.join`` - - Examples: - - >>> from scrapy.loader.processors import Join - >>> proc = Join() - >>> proc(['one', 'two', 'three']) - 'one two three' - >>> proc = Join('<br>') - >>> proc(['one', 'two', 'three']) - 'one<br>two<br>three' - -.. class:: Compose(*functions, **default_loader_context) - - A processor which is constructed from the composition of the given - functions. This means that each input value of this processor is passed to - the first function, and the result of that function is passed to the second - function, and so on, until the last function returns the output value of - this processor. - - By default, stop process on ``None`` value. This behaviour can be changed by - passing keyword argument ``stop_on_none=False``. - - Example: - - >>> from scrapy.loader.processors import Compose - >>> proc = Compose(lambda v: v[0], str.upper) - >>> proc(['hello', 'world']) - 'HELLO' - - Each function can optionally receive a ``loader_context`` parameter. For - those which do, this processor will pass the currently active :ref:`Loader - context <topics-loaders-context>` through that parameter. - - The keyword arguments passed in the ``__init__`` method are used as the default - Loader context values passed to each function call. However, the final - Loader context values passed to functions are overridden with the currently - active Loader context accessible through the :meth:`ItemLoader.context` - attribute. - -.. class:: MapCompose(*functions, **default_loader_context) - - A processor which is constructed from the composition of the given - functions, similar to the :class:`Compose` processor. The difference with - this processor is the way internal results are passed among functions, - which is as follows: - - The input value of this processor is *iterated* and the first function is - applied to each element. The results of these function calls (one for each element) - are concatenated to construct a new iterable, which is then used to apply the - second function, and so on, until the last function is applied to each - value of the list of values collected so far. The output values of the last - function are concatenated together to produce the output of this processor. - - Each particular function can return a value or a list of values, which is - flattened with the list of values returned by the same function applied to - the other input values. The functions can also return ``None`` in which - case the output of that function is ignored for further processing over the - chain. - - This processor provides a convenient way to compose functions that only - work with single values (instead of iterables). For this reason the - :class:`MapCompose` processor is typically used as input processor, since - data is often extracted using the - :meth:`~scrapy.selector.Selector.extract` method of :ref:`selectors - <topics-selectors>`, which returns a list of unicode strings. - - The example below should clarify how it works: - - >>> def filter_world(x): - ... return None if x == 'world' else x - ... - >>> from scrapy.loader.processors import MapCompose - >>> proc = MapCompose(filter_world, str.upper) - >>> proc(['hello', 'world', 'this', 'is', 'scrapy']) - ['HELLO, 'THIS', 'IS', 'SCRAPY'] - - As with the Compose processor, functions can receive Loader contexts, and - ``__init__`` method keyword arguments are used as default context values. See - :class:`Compose` processor for more info. - -.. class:: SelectJmes(json_path) - - Queries the value using the json path provided to the ``__init__`` method and returns the output. - Requires jmespath (https://github.com/jmespath/jmespath.py) to run. - This processor takes only one input at a time. - - Example: - - >>> from scrapy.loader.processors import SelectJmes, Compose, MapCompose - >>> proc = SelectJmes("foo") #for direct use on lists and dictionaries - >>> proc({'foo': 'bar'}) - 'bar' - >>> proc({'foo': {'bar': 'baz'}}) - {'bar': 'baz'} - - Working with Json: - - >>> import json - >>> proc_single_json_str = Compose(json.loads, SelectJmes("foo")) - >>> proc_single_json_str('{"foo": "bar"}') - 'bar' - >>> proc_json_list = Compose(json.loads, MapCompose(SelectJmes('foo'))) - >>> proc_json_list('[{"foo":"bar"}, {"baz":"tar"}]') - ['bar'] +.. _itemloaders: https://itemloaders.readthedocs.io/en/latest/ +.. _processors: https://itemloaders.readthedocs.io/en/latest/built-in-processors.html diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index 18f57945f..014951a8e 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -3,217 +3,86 @@ Item Loader See documentation in docs/topics/loaders.rst """ -from collections import defaultdict -from contextlib import suppress - -from itemadapter import ItemAdapter +import itemloaders from scrapy.item import Item -from scrapy.loader.common import wrap_loader_context -from scrapy.loader.processors import Identity from scrapy.selector import Selector -from scrapy.utils.misc import arg_to_iter, extract_regex -from scrapy.utils.python import flatten -def unbound_method(method): +class ItemLoader(itemloaders.ItemLoader): """ - Allow to use single-argument functions as input or output processors - (no need to define an unused first 'self' argument) + A user-friendly abstraction to populate an :ref:`item <topics-items>` with data + by applying :ref:`field processors <topics-loaders-processors>` to scraped data. + When instantiated with a ``selector`` or a ``response`` it supports + data extraction from web pages using :ref:`selectors <topics-selectors>`. + + :param item: The item instance to populate using subsequent calls to + :meth:`~ItemLoader.add_xpath`, :meth:`~ItemLoader.add_css`, + or :meth:`~ItemLoader.add_value`. + :type item: scrapy.item.Item + + :param selector: The selector to extract data from, when using the + :meth:`add_xpath`, :meth:`add_css`, :meth:`replace_xpath`, or + :meth:`replace_css` method. + :type selector: :class:`~scrapy.selector.Selector` object + + :param response: The response used to construct the selector using the + :attr:`default_selector_class`, unless the selector argument is given, + in which case this argument is ignored. + :type response: :class:`~scrapy.http.Response` object + + If no item is given, one is instantiated automatically using the class in + :attr:`default_item_class`. + + The item, selector, response and remaining keyword arguments are + assigned to the Loader context (accessible through the :attr:`context` attribute). + + .. attribute:: item + + The item object being parsed by this Item Loader. + This is mostly used as a property so, when attempting to override this + value, you may want to check out :attr:`default_item_class` first. + + .. attribute:: context + + The currently active :ref:`Context <loaders-context>` of this Item Loader. + + .. attribute:: default_item_class + + An :ref:`item <topics-items>` class (or factory), used to instantiate + items when not given in the ``__init__`` method. + + .. attribute:: default_input_processor + + The default input processor to use for those fields which don't specify + one. + + .. attribute:: default_output_processor + + The default output processor to use for those fields which don't specify + one. + + .. attribute:: default_selector_class + + The class used to construct the :attr:`selector` of this + :class:`ItemLoader`, if only a response is given in the ``__init__`` method. + If a selector is given in the ``__init__`` method this attribute is ignored. + This attribute is sometimes overridden in subclasses. + + .. attribute:: selector + + The :class:`~scrapy.selector.Selector` object to extract data from. + It's either the selector given in the ``__init__`` method or one created from + the response given in the ``__init__`` method using the + :attr:`default_selector_class`. This attribute is meant to be + read-only. """ - with suppress(AttributeError): - if '.' not in method.__qualname__: - return method.__func__ - return method - - -class ItemLoader: default_item_class = Item - default_input_processor = Identity() - default_output_processor = Identity() default_selector_class = Selector def __init__(self, item=None, selector=None, response=None, parent=None, **context): if selector is None and response is not None: selector = self.default_selector_class(response) - self.selector = selector - context.update(selector=selector, response=response) - if item is None: - item = self.default_item_class() - self.context = context - self.parent = parent - self._local_item = context['item'] = item - self._local_values = defaultdict(list) - # values from initial item - for field_name, value in ItemAdapter(item).items(): - self._values[field_name] += arg_to_iter(value) - - @property - def _values(self): - if self.parent is not None: - return self.parent._values - else: - return self._local_values - - @property - def item(self): - if self.parent is not None: - return self.parent.item - else: - return self._local_item - - def nested_xpath(self, xpath, **context): - selector = self.selector.xpath(xpath) - context.update(selector=selector) - subloader = self.__class__( - item=self.item, parent=self, **context - ) - return subloader - - def nested_css(self, css, **context): - selector = self.selector.css(css) - context.update(selector=selector) - subloader = self.__class__( - item=self.item, parent=self, **context - ) - return subloader - - def add_value(self, field_name, value, *processors, **kw): - value = self.get_value(value, *processors, **kw) - if value is None: - return - if not field_name: - for k, v in value.items(): - self._add_value(k, v) - else: - self._add_value(field_name, value) - - def replace_value(self, field_name, value, *processors, **kw): - value = self.get_value(value, *processors, **kw) - if value is None: - return - if not field_name: - for k, v in value.items(): - self._replace_value(k, v) - else: - self._replace_value(field_name, value) - - def _add_value(self, field_name, value): - value = arg_to_iter(value) - processed_value = self._process_input_value(field_name, value) - if processed_value: - self._values[field_name] += arg_to_iter(processed_value) - - def _replace_value(self, field_name, value): - self._values.pop(field_name, None) - self._add_value(field_name, value) - - def get_value(self, value, *processors, **kw): - regex = kw.get('re', None) - if regex: - value = arg_to_iter(value) - value = flatten(extract_regex(regex, x) for x in value) - - for proc in processors: - if value is None: - break - _proc = proc - proc = wrap_loader_context(proc, self.context) - try: - value = proc(value) - except Exception as e: - raise ValueError("Error with processor %s value=%r error='%s: %s'" % - (_proc.__class__.__name__, value, - type(e).__name__, str(e))) - return value - - def load_item(self): - adapter = ItemAdapter(self.item) - for field_name in tuple(self._values): - value = self.get_output_value(field_name) - if value is not None: - adapter[field_name] = value - return adapter.item - - def get_output_value(self, field_name): - proc = self.get_output_processor(field_name) - proc = wrap_loader_context(proc, self.context) - try: - return proc(self._values[field_name]) - except Exception as e: - raise ValueError("Error with output processor: field=%r value=%r error='%s: %s'" % - (field_name, self._values[field_name], type(e).__name__, str(e))) - - def get_collected_values(self, field_name): - return self._values[field_name] - - def get_input_processor(self, field_name): - proc = getattr(self, '%s_in' % field_name, None) - if not proc: - proc = self._get_item_field_attr(field_name, 'input_processor', - self.default_input_processor) - return unbound_method(proc) - - def get_output_processor(self, field_name): - proc = getattr(self, '%s_out' % field_name, None) - if not proc: - proc = self._get_item_field_attr(field_name, 'output_processor', - self.default_output_processor) - return unbound_method(proc) - - def _process_input_value(self, field_name, value): - proc = self.get_input_processor(field_name) - _proc = proc - proc = wrap_loader_context(proc, self.context) - try: - return proc(value) - except Exception as e: - raise ValueError( - "Error with input processor %s: field=%r value=%r " - "error='%s: %s'" % (_proc.__class__.__name__, field_name, - value, type(e).__name__, str(e))) - - def _get_item_field_attr(self, field_name, key, default=None): - field_meta = ItemAdapter(self.item).get_field_meta(field_name) - return field_meta.get(key, default) - - def _check_selector_method(self): - if self.selector is None: - raise RuntimeError("To use XPath or CSS selectors, " - "%s must be instantiated with a selector " - "or a response" % self.__class__.__name__) - - def add_xpath(self, field_name, xpath, *processors, **kw): - values = self._get_xpathvalues(xpath, **kw) - self.add_value(field_name, values, *processors, **kw) - - def replace_xpath(self, field_name, xpath, *processors, **kw): - values = self._get_xpathvalues(xpath, **kw) - self.replace_value(field_name, values, *processors, **kw) - - def get_xpath(self, xpath, *processors, **kw): - values = self._get_xpathvalues(xpath, **kw) - return self.get_value(values, *processors, **kw) - - def _get_xpathvalues(self, xpaths, **kw): - self._check_selector_method() - xpaths = arg_to_iter(xpaths) - return flatten(self.selector.xpath(xpath).getall() for xpath in xpaths) - - def add_css(self, field_name, css, *processors, **kw): - values = self._get_cssvalues(css, **kw) - self.add_value(field_name, values, *processors, **kw) - - def replace_css(self, field_name, css, *processors, **kw): - values = self._get_cssvalues(css, **kw) - self.replace_value(field_name, values, *processors, **kw) - - def get_css(self, css, *processors, **kw): - values = self._get_cssvalues(css, **kw) - return self.get_value(values, *processors, **kw) - - def _get_cssvalues(self, csss, **kw): - self._check_selector_method() - csss = arg_to_iter(csss) - return flatten(self.selector.css(css).getall() for css in csss) + context.update(response=response) + super().__init__(item=item, selector=selector, parent=parent, **context) diff --git a/scrapy/loader/common.py b/scrapy/loader/common.py index 42f8de636..3b8a6ee94 100644 --- a/scrapy/loader/common.py +++ b/scrapy/loader/common.py @@ -1,14 +1,21 @@ """Common functions used in Item Loaders code""" -from functools import partial -from scrapy.utils.python import get_func_args +import warnings + +from itemloaders import common + +from scrapy.utils.deprecate import ScrapyDeprecationWarning def wrap_loader_context(function, context): """Wrap functions that receive loader_context to contain the context "pre-loaded" and expose a interface that receives only one argument """ - if 'loader_context' in get_func_args(function): - return partial(function, loader_context=context) - else: - return function + warnings.warn( + "scrapy.loader.common.wrap_loader_context has moved to a new library." + "Please update your reference to itemloaders.common.wrap_loader_context", + ScrapyDeprecationWarning, + stacklevel=2 + ) + + return common.wrap_loader_context(function, context) diff --git a/scrapy/loader/processors.py b/scrapy/loader/processors.py index a7be65609..51fbd19eb 100644 --- a/scrapy/loader/processors.py +++ b/scrapy/loader/processors.py @@ -3,102 +3,19 @@ This module provides some commonly used processors for Item Loaders. See documentation in docs/topics/loaders.rst """ -from collections import ChainMap +from itemloaders import processors -from scrapy.utils.misc import arg_to_iter -from scrapy.loader.common import wrap_loader_context +from scrapy.utils.deprecate import create_deprecated_class -class MapCompose: +MapCompose = create_deprecated_class('MapCompose', processors.MapCompose) - def __init__(self, *functions, **default_loader_context): - self.functions = functions - self.default_loader_context = default_loader_context +Compose = create_deprecated_class('Compose', processors.Compose) - def __call__(self, value, loader_context=None): - values = arg_to_iter(value) - if loader_context: - context = ChainMap(loader_context, self.default_loader_context) - else: - context = self.default_loader_context - wrapped_funcs = [wrap_loader_context(f, context) for f in self.functions] - for func in wrapped_funcs: - next_values = [] - for v in values: - try: - next_values += arg_to_iter(func(v)) - except Exception as e: - raise ValueError("Error in MapCompose with " - "%s value=%r error='%s: %s'" % - (str(func), value, type(e).__name__, - str(e))) - values = next_values - return values +TakeFirst = create_deprecated_class('TakeFirst', processors.TakeFirst) +Identity = create_deprecated_class('Identity', processors.Identity) -class Compose: +SelectJmes = create_deprecated_class('SelectJmes', processors.SelectJmes) - def __init__(self, *functions, **default_loader_context): - self.functions = functions - self.stop_on_none = default_loader_context.get('stop_on_none', True) - self.default_loader_context = default_loader_context - - def __call__(self, value, loader_context=None): - if loader_context: - context = ChainMap(loader_context, self.default_loader_context) - else: - context = self.default_loader_context - wrapped_funcs = [wrap_loader_context(f, context) for f in self.functions] - for func in wrapped_funcs: - if value is None and self.stop_on_none: - break - try: - value = func(value) - except Exception as e: - raise ValueError("Error in Compose with " - "%s value=%r error='%s: %s'" % - (str(func), value, type(e).__name__, str(e))) - return value - - -class TakeFirst: - - def __call__(self, values): - for value in values: - if value is not None and value != '': - return value - - -class Identity: - - def __call__(self, values): - return values - - -class SelectJmes: - """ - Query the input string for the jmespath (given at instantiation), - and return the answer - Requires : jmespath(https://github.com/jmespath/jmespath) - Note: SelectJmes accepts only one input element at a time. - """ - def __init__(self, json_path): - self.json_path = json_path - import jmespath - self.compiled_path = jmespath.compile(self.json_path) - - def __call__(self, value): - """Query value for the jmespath query and return answer - :param value: a data structure (dict, list) to extract from - :return: Element extracted according to jmespath query - """ - return self.compiled_path.search(value) - - -class Join: - - def __init__(self, separator=u' '): - self.separator = separator - - def __call__(self, values): - return self.separator.join(values) +Join = create_deprecated_class('Join', processors.Join) diff --git a/scrapy/utils/misc.py b/scrapy/utils/misc.py index 8e5fde246..d6966be8e 100644 --- a/scrapy/utils/misc.py +++ b/scrapy/utils/misc.py @@ -15,6 +15,7 @@ from w3lib.html import replace_entities from scrapy.utils.datatypes import LocalWeakReferencedCache from scrapy.utils.python import flatten, to_unicode from scrapy.item import _BaseItem +from scrapy.utils.deprecate import ScrapyDeprecationWarning _ITERABLE_SINGLE_VALUES = dict, _BaseItem, str, bytes @@ -86,6 +87,11 @@ def extract_regex(regex, text, encoding='utf-8'): * if the regex contains multiple numbered groups, all those will be returned (flattened) * if the regex doesn't contain any group the entire regex matching is returned """ + warnings.warn( + "scrapy.utils.misc.extract_regex has moved to parsel.utils.extract_regex.", + ScrapyDeprecationWarning, + stacklevel=2 + ) if isinstance(regex, str): regex = re.compile(regex, re.UNICODE) diff --git a/setup.cfg b/setup.cfg index a9138c1c0..46a3d13fc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -118,6 +118,9 @@ ignore_errors = True [mypy-tests.test_loader] ignore_errors = True +[mypy-tests.test_loader_deprecated] +ignore_errors = True + [mypy-tests.test_pipeline_crawl] ignore_errors = True diff --git a/setup.py b/setup.py index 5a99fd1bf..f8d9b491b 100644 --- a/setup.py +++ b/setup.py @@ -71,6 +71,7 @@ setup( 'Twisted>=17.9.0', 'cryptography>=2.0', 'cssselect>=0.9.1', + 'itemloaders>=1.0.1', 'lxml>=3.5.0', 'parsel>=1.5.0', 'PyDispatcher>=2.0.5', diff --git a/tests/requirements-py3.txt b/tests/requirements-py3.txt index dacb86e56..0551b1e95 100644 --- a/tests/requirements-py3.txt +++ b/tests/requirements-py3.txt @@ -1,7 +1,6 @@ # Tests requirements attrs dataclasses; python_version == '3.6' -jmespath mitmproxy; python_version >= '3.6' mitmproxy<4.0.0; python_version < '3.6' # https://github.com/pytest-dev/pytest-twisted/issues/93 diff --git a/tests/test_loader.py b/tests/test_loader.py index 8a9c6fca9..581183625 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -1,14 +1,12 @@ -from functools import partial import unittest import attr from itemadapter import ItemAdapter +from itemloaders.processors import Compose, Identity, MapCompose, TakeFirst from scrapy.http import HtmlResponse from scrapy.item import Item, Field from scrapy.loader import ItemLoader -from scrapy.loader.processors import (Compose, Identity, Join, - MapCompose, SelectJmes, TakeFirst) from scrapy.selector import Selector @@ -69,6 +67,10 @@ def processor_with_args(value, other=None, loader_context=None): class BasicItemLoaderTest(unittest.TestCase): + def test_add_value_on_unknown_field(self): + il = TestItemLoader() + self.assertRaises(KeyError, il.add_value, 'wrong_field', [u'lala', u'lolo']) + def test_load_item_using_default_loader(self): i = TestItem() i['summary'] = u'lala' @@ -85,391 +87,6 @@ class BasicItemLoaderTest(unittest.TestCase): item = il.load_item() self.assertEqual(item['name'], [u'Marta']) - def test_load_item_ignore_none_field_values(self): - def validate_sku(value): - # Let's assume a SKU is only digits. - if value.isdigit(): - return value - - class MyLoader(ItemLoader): - name_out = Compose(lambda vs: vs[0]) # take first which allows empty values - price_out = Compose(TakeFirst(), float) - sku_out = Compose(TakeFirst(), validate_sku) - - valid_fragment = u'SKU: 1234' - invalid_fragment = u'SKU: not available' - sku_re = 'SKU: (.+)' - - il = MyLoader(item={}) - # Should not return "sku: None". - il.add_value('sku', [invalid_fragment], re=sku_re) - # Should not ignore empty values. - il.add_value('name', u'') - il.add_value('price', [u'0']) - self.assertEqual(il.load_item(), { - 'name': u'', - 'price': 0.0, - }) - - il.replace_value('sku', [valid_fragment], re=sku_re) - self.assertEqual(il.load_item()['sku'], u'1234') - - def test_self_referencing_loader(self): - class MyLoader(ItemLoader): - url_out = TakeFirst() - - def img_url_out(self, values): - return (self.get_output_value('url') or '') + values[0] - - il = MyLoader(item={}) - il.add_value('url', 'http://example.com/') - il.add_value('img_url', '1234.png') - self.assertEqual(il.load_item(), { - 'url': 'http://example.com/', - 'img_url': 'http://example.com/1234.png', - }) - - il = MyLoader(item={}) - il.add_value('img_url', '1234.png') - self.assertEqual(il.load_item(), { - 'img_url': '1234.png', - }) - - def test_add_value(self): - il = TestItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_collected_values('name'), [u'Marta']) - self.assertEqual(il.get_output_value('name'), [u'Marta']) - il.add_value('name', u'pepe') - self.assertEqual(il.get_collected_values('name'), [u'Marta', u'Pepe']) - self.assertEqual(il.get_output_value('name'), [u'Marta', u'Pepe']) - - # test add object value - il.add_value('summary', {'key': 1}) - self.assertEqual(il.get_collected_values('summary'), [{'key': 1}]) - - il.add_value(None, u'Jim', lambda x: {'name': x}) - self.assertEqual(il.get_collected_values('name'), [u'Marta', u'Pepe', u'Jim']) - - def test_add_zero(self): - il = NameItemLoader() - il.add_value('name', 0) - self.assertEqual(il.get_collected_values('name'), [0]) - - def test_replace_value(self): - il = TestItemLoader() - il.replace_value('name', u'marta') - self.assertEqual(il.get_collected_values('name'), [u'Marta']) - self.assertEqual(il.get_output_value('name'), [u'Marta']) - il.replace_value('name', u'pepe') - self.assertEqual(il.get_collected_values('name'), [u'Pepe']) - self.assertEqual(il.get_output_value('name'), [u'Pepe']) - - il.replace_value(None, u'Jim', lambda x: {'name': x}) - self.assertEqual(il.get_collected_values('name'), [u'Jim']) - - def test_get_value(self): - il = NameItemLoader() - self.assertEqual(u'FOO', il.get_value([u'foo', u'bar'], TakeFirst(), str.upper)) - self.assertEqual([u'foo', u'bar'], il.get_value([u'name:foo', u'name:bar'], re=u'name:(.*)$')) - self.assertEqual(u'foo', il.get_value([u'name:foo', u'name:bar'], TakeFirst(), re=u'name:(.*)$')) - - il.add_value('name', [u'name:foo', u'name:bar'], TakeFirst(), re=u'name:(.*)$') - self.assertEqual([u'foo'], il.get_collected_values('name')) - il.replace_value('name', u'name:bar', re=u'name:(.*)$') - self.assertEqual([u'bar'], il.get_collected_values('name')) - - def test_iter_on_input_processor_input(self): - class NameFirstItemLoader(NameItemLoader): - name_in = TakeFirst() - - il = NameFirstItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_collected_values('name'), [u'marta']) - il = NameFirstItemLoader() - il.add_value('name', [u'marta', u'jose']) - self.assertEqual(il.get_collected_values('name'), [u'marta']) - - il = NameFirstItemLoader() - il.replace_value('name', u'marta') - self.assertEqual(il.get_collected_values('name'), [u'marta']) - il = NameFirstItemLoader() - il.replace_value('name', [u'marta', u'jose']) - self.assertEqual(il.get_collected_values('name'), [u'marta']) - - il = NameFirstItemLoader() - il.add_value('name', u'marta') - il.add_value('name', [u'jose', u'pedro']) - self.assertEqual(il.get_collected_values('name'), [u'marta', u'jose']) - - def test_map_compose_filter(self): - def filter_world(x): - return None if x == 'world' else x - - proc = MapCompose(filter_world, str.upper) - self.assertEqual(proc(['hello', 'world', 'this', 'is', 'scrapy']), - ['HELLO', 'THIS', 'IS', 'SCRAPY']) - - def test_map_compose_filter_multil(self): - class TestItemLoader(NameItemLoader): - name_in = MapCompose(lambda v: v.title(), lambda v: v[:-1]) - - il = TestItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'Mart']) - item = il.load_item() - self.assertEqual(item['name'], [u'Mart']) - - def test_default_input_processor(self): - il = DefaultedItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'mart']) - - def test_inherited_default_input_processor(self): - class InheritDefaultedItemLoader(DefaultedItemLoader): - pass - - il = InheritDefaultedItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'mart']) - - def test_input_processor_inheritance(self): - class ChildItemLoader(TestItemLoader): - url_in = MapCompose(lambda v: v.lower()) - - il = ChildItemLoader() - il.add_value('url', u'HTTP://scrapy.ORG') - self.assertEqual(il.get_output_value('url'), [u'http://scrapy.org']) - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'Marta']) - - class ChildChildItemLoader(ChildItemLoader): - url_in = MapCompose(lambda v: v.upper()) - summary_in = MapCompose(lambda v: v) - - il = ChildChildItemLoader() - il.add_value('url', u'http://scrapy.org') - self.assertEqual(il.get_output_value('url'), [u'HTTP://SCRAPY.ORG']) - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'Marta']) - - def test_empty_map_compose(self): - class IdentityDefaultedItemLoader(DefaultedItemLoader): - name_in = MapCompose() - - il = IdentityDefaultedItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'marta']) - - def test_identity_input_processor(self): - class IdentityDefaultedItemLoader(DefaultedItemLoader): - name_in = Identity() - - il = IdentityDefaultedItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'marta']) - - def test_extend_custom_input_processors(self): - class ChildItemLoader(TestItemLoader): - name_in = MapCompose(TestItemLoader.name_in, str.swapcase) - - il = ChildItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'mARTA']) - - def test_extend_default_input_processors(self): - class ChildDefaultedItemLoader(DefaultedItemLoader): - name_in = MapCompose(DefaultedItemLoader.default_input_processor, str.swapcase) - - il = ChildDefaultedItemLoader() - il.add_value('name', u'marta') - self.assertEqual(il.get_output_value('name'), [u'MART']) - - def test_output_processor_using_function(self): - il = TestItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) - - class TakeFirstItemLoader(TestItemLoader): - name_out = u" ".join - - il = TakeFirstItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), u'Mar Ta') - - def test_output_processor_error(self): - class TestItemLoader(ItemLoader): - default_item_class = TestItem - name_out = MapCompose(float) - - il = TestItemLoader() - il.add_value('name', [u'$10']) - try: - float(u'$10') - except Exception as e: - expected_exc_str = str(e) - - exc = None - try: - il.load_item() - except Exception as e: - exc = e - assert isinstance(exc, ValueError) - s = str(exc) - assert 'name' in s, s - assert '$10' in s, s - assert 'ValueError' in s, s - assert expected_exc_str in s, s - - def test_output_processor_using_classes(self): - il = TestItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) - - class TakeFirstItemLoader(TestItemLoader): - name_out = Join() - - il = TakeFirstItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), u'Mar Ta') - - class TakeFirstItemLoader(TestItemLoader): - name_out = Join("<br>") - - il = TakeFirstItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), u'Mar<br>Ta') - - def test_default_output_processor(self): - il = TestItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) - - class LalaItemLoader(TestItemLoader): - default_output_processor = Identity() - - il = LalaItemLoader() - il.add_value('name', [u'mar', u'ta']) - self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) - - def test_loader_context_on_declaration(self): - class ChildItemLoader(TestItemLoader): - url_in = MapCompose(processor_with_args, key=u'val') - - il = ChildItemLoader() - il.add_value('url', u'text') - self.assertEqual(il.get_output_value('url'), ['val']) - il.replace_value('url', u'text2') - self.assertEqual(il.get_output_value('url'), ['val']) - - def test_loader_context_on_instantiation(self): - class ChildItemLoader(TestItemLoader): - url_in = MapCompose(processor_with_args) - - il = ChildItemLoader(key=u'val') - il.add_value('url', u'text') - self.assertEqual(il.get_output_value('url'), ['val']) - il.replace_value('url', u'text2') - self.assertEqual(il.get_output_value('url'), ['val']) - - def test_loader_context_on_assign(self): - class ChildItemLoader(TestItemLoader): - url_in = MapCompose(processor_with_args) - - il = ChildItemLoader() - il.context['key'] = u'val' - il.add_value('url', u'text') - self.assertEqual(il.get_output_value('url'), ['val']) - il.replace_value('url', u'text2') - self.assertEqual(il.get_output_value('url'), ['val']) - - def test_item_passed_to_input_processor_functions(self): - def processor(value, loader_context): - return loader_context['item']['name'] - - class ChildItemLoader(TestItemLoader): - url_in = MapCompose(processor) - - it = TestItem(name='marta') - il = ChildItemLoader(item=it) - il.add_value('url', u'text') - self.assertEqual(il.get_output_value('url'), ['marta']) - il.replace_value('url', u'text2') - self.assertEqual(il.get_output_value('url'), ['marta']) - - def test_add_value_on_unknown_field(self): - il = TestItemLoader() - self.assertRaises(KeyError, il.add_value, 'wrong_field', [u'lala', u'lolo']) - - def test_compose_processor(self): - class TestItemLoader(NameItemLoader): - name_out = Compose(lambda v: v[0], lambda v: v.title(), lambda v: v[:-1]) - - il = TestItemLoader() - il.add_value('name', [u'marta', u'other']) - self.assertEqual(il.get_output_value('name'), u'Mart') - item = il.load_item() - self.assertEqual(item['name'], u'Mart') - - def test_partial_processor(self): - def join(values, sep=None, loader_context=None, ignored=None): - if sep is not None: - return sep.join(values) - elif loader_context and 'sep' in loader_context: - return loader_context['sep'].join(values) - else: - return ''.join(values) - - class TestItemLoader(NameItemLoader): - name_out = Compose(partial(join, sep='+')) - url_out = Compose(partial(join, loader_context={'sep': '.'})) - summary_out = Compose(partial(join, ignored='foo')) - - il = TestItemLoader() - il.add_value('name', [u'rabbit', u'hole']) - il.add_value('url', [u'rabbit', u'hole']) - il.add_value('summary', [u'rabbit', u'hole']) - item = il.load_item() - self.assertEqual(item['name'], u'rabbit+hole') - self.assertEqual(item['url'], u'rabbit.hole') - self.assertEqual(item['summary'], u'rabbithole') - - def test_error_input_processor(self): - class TestItem(Item): - name = Field() - - class TestItemLoader(ItemLoader): - default_item_class = TestItem - name_in = MapCompose(float) - - il = TestItemLoader() - self.assertRaises(ValueError, il.add_value, 'name', - [u'marta', u'other']) - - def test_error_output_processor(self): - class TestItem(Item): - name = Field() - - class TestItemLoader(ItemLoader): - default_item_class = TestItem - name_out = Compose(Join(), float) - - il = TestItemLoader() - il.add_value('name', u'marta') - with self.assertRaises(ValueError): - il.load_item() - - def test_error_processor_as_argument(self): - class TestItem(Item): - name = Field() - - class TestItemLoader(ItemLoader): - default_item_class = TestItem - - il = TestItemLoader() - self.assertRaises(ValueError, il.add_value, 'name', - [u'marta', u'other'], Compose(float)) - class InitializationTestMixin: @@ -587,41 +204,6 @@ class BaseNoInputReprocessingLoader(ItemLoader): title_out = TakeFirst() -class NoInputReprocessingDictLoader(BaseNoInputReprocessingLoader): - default_item_class = dict - - -class NoInputReprocessingFromDictTest(unittest.TestCase): - """ - Loaders initialized from loaded items must not reprocess fields (dict instances) - """ - def test_avoid_reprocessing_with_initial_values_single(self): - il = NoInputReprocessingDictLoader(item=dict(title='foo')) - il_loaded = il.load_item() - self.assertEqual(il_loaded, dict(title='foo')) - self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='foo')) - - def test_avoid_reprocessing_with_initial_values_list(self): - il = NoInputReprocessingDictLoader(item=dict(title=['foo', 'bar'])) - il_loaded = il.load_item() - self.assertEqual(il_loaded, dict(title='foo')) - self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='foo')) - - def test_avoid_reprocessing_without_initial_values_single(self): - il = NoInputReprocessingDictLoader() - il.add_value('title', 'foo') - il_loaded = il.load_item() - self.assertEqual(il_loaded, dict(title='FOO')) - self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='FOO')) - - def test_avoid_reprocessing_without_initial_values_list(self): - il = NoInputReprocessingDictLoader() - il.add_value('title', ['foo', 'bar']) - il_loaded = il.load_item() - self.assertEqual(il_loaded, dict(title='FOO')) - self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='FOO')) - - class NoInputReprocessingItem(Item): title = Field() @@ -661,25 +243,6 @@ class NoInputReprocessingFromItemTest(unittest.TestCase): self.assertEqual(NoInputReprocessingItemLoader(item=il_loaded).load_item(), {'title': 'FOO'}) -class TestOutputProcessorDict(unittest.TestCase): - def test_output_processor(self): - - class TempDict(dict): - def __init__(self, *args, **kwargs): - super(TempDict, self).__init__(self, *args, **kwargs) - self.setdefault('temp', 0.3) - - class TempLoader(ItemLoader): - default_item_class = TempDict - default_input_processor = Identity() - default_output_processor = Compose(TakeFirst()) - - loader = TempLoader() - item = loader.load_item() - self.assertIsInstance(item, TempDict) - self.assertEqual(dict(item), {'temp': 0.3}) - - class TestOutputProcessorItem(unittest.TestCase): def test_output_processor(self): @@ -701,49 +264,6 @@ class TestOutputProcessorItem(unittest.TestCase): self.assertEqual(dict(item), {'temp': 0.3}) -class ProcessorsTest(unittest.TestCase): - - def test_take_first(self): - proc = TakeFirst() - self.assertEqual(proc([None, '', 'hello', 'world']), 'hello') - self.assertEqual(proc([None, '', 0, 'hello', 'world']), 0) - - def test_identity(self): - proc = Identity() - self.assertEqual(proc([None, '', 'hello', 'world']), - [None, '', 'hello', 'world']) - - def test_join(self): - proc = Join() - self.assertRaises(TypeError, proc, [None, '', 'hello', 'world']) - self.assertEqual(proc(['', 'hello', 'world']), u' hello world') - self.assertEqual(proc(['hello', 'world']), u'hello world') - self.assertIsInstance(proc(['hello', 'world']), str) - - def test_compose(self): - proc = Compose(lambda v: v[0], str.upper) - self.assertEqual(proc(['hello', 'world']), 'HELLO') - proc = Compose(str.upper) - self.assertEqual(proc(None), None) - proc = Compose(str.upper, stop_on_none=False) - self.assertRaises(ValueError, proc, None) - proc = Compose(str.upper, lambda x: x + 1) - self.assertRaises(ValueError, proc, 'hello') - - def test_mapcompose(self): - def filter_world(x): - return None if x == 'world' else x - proc = MapCompose(filter_world, str.upper) - self.assertEqual(proc([u'hello', u'world', u'this', u'is', u'scrapy']), - [u'HELLO', u'THIS', u'IS', u'SCRAPY']) - proc = MapCompose(filter_world, str.upper) - self.assertEqual(proc(None), []) - proc = MapCompose(filter_world, str.upper) - self.assertRaises(ValueError, proc, [1]) - proc = MapCompose(filter_world, lambda x: x + 1) - self.assertRaises(ValueError, proc, 'hello') - - class SelectortemLoaderTest(unittest.TestCase): response = HtmlResponse(url="", encoding='utf-8', body=b""" <html> @@ -921,6 +441,7 @@ class SubselectorLoaderTest(unittest.TestCase): def test_nested_xpath(self): l = NestedItemLoader(response=self.response) + nl = l.nested_xpath("//header") nl.add_xpath('name', 'div/text()') nl.add_css('name_div', '#id') @@ -998,31 +519,6 @@ class SubselectorLoaderTest(unittest.TestCase): self.assertEqual(item['image'], [u'/images/logo.png']) -class SelectJmesTestCase(unittest.TestCase): - test_list_equals = { - 'simple': ('foo.bar', {"foo": {"bar": "baz"}}, "baz"), - 'invalid': ('foo.bar.baz', {"foo": {"bar": "baz"}}, None), - 'top_level': ('foo', {"foo": {"bar": "baz"}}, {"bar": "baz"}), - 'double_vs_single_quote_string': ('foo.bar', {"foo": {"bar": "baz"}}, "baz"), - 'dict': ( - 'foo.bar[*].name', - {"foo": {"bar": [{"name": "one"}, {"name": "two"}]}}, - ['one', 'two'] - ), - 'list': ('[1]', [1, 2], 2) - } - - def test_output(self): - for l in self.test_list_equals: - expr, test_list, expected = self.test_list_equals[l] - test = SelectJmes(expr)(test_list) - self.assertEqual( - test, - expected, - msg='test "{}" got {} expected {}'.format(l, test, expected) - ) - - # Functions as processors def function_processor_strip(iterable): @@ -1044,12 +540,6 @@ class FunctionProcessorItemLoader(ItemLoader): default_item_class = FunctionProcessorItem -class FunctionProcessorDictLoader(ItemLoader): - default_item_class = dict - foo_in = function_processor_strip - foo_out = function_processor_upper - - class FunctionProcessorTestCase(unittest.TestCase): def test_processor_defined_in_item(self): @@ -1061,15 +551,6 @@ class FunctionProcessorTestCase(unittest.TestCase): {'foo': ['BAR', 'ASDF', 'QWERTY']} ) - def test_processor_defined_in_item_loader(self): - lo = FunctionProcessorDictLoader() - lo.add_value('foo', ' bar ') - lo.add_value('foo', [' asdf ', ' qwerty ']) - self.assertEqual( - dict(lo.load_item()), - {'foo': ['BAR', 'ASDF', 'QWERTY']} - ) - if __name__ == "__main__": unittest.main() diff --git a/tests/test_loader_deprecated.py b/tests/test_loader_deprecated.py new file mode 100644 index 000000000..d0a59e8cd --- /dev/null +++ b/tests/test_loader_deprecated.py @@ -0,0 +1,720 @@ +""" +These tests are kept as references from the ones that were ported to a itemloaders library. +Once we remove the references from scrapy, we can remove these tests. +""" + +import unittest +import warnings +from functools import partial + +from itemloaders.processors import (Compose, Identity, Join, + MapCompose, SelectJmes, TakeFirst) + +from scrapy.item import Item, Field +from scrapy.loader import ItemLoader +from scrapy.loader.common import wrap_loader_context +from scrapy.utils.deprecate import ScrapyDeprecationWarning +from scrapy.utils.misc import extract_regex + + +# test items +class NameItem(Item): + name = Field() + + +class TestItem(NameItem): + url = Field() + summary = Field() + + +# test item loaders +class NameItemLoader(ItemLoader): + default_item_class = TestItem + + +class TestItemLoader(NameItemLoader): + name_in = MapCompose(lambda v: v.title()) + + +class DefaultedItemLoader(NameItemLoader): + default_input_processor = MapCompose(lambda v: v[:-1]) + + +# test processors +def processor_with_args(value, other=None, loader_context=None): + if 'key' in loader_context: + return loader_context['key'] + return value + + +class BasicItemLoaderTest(unittest.TestCase): + + def test_load_item_using_default_loader(self): + i = TestItem() + i['summary'] = u'lala' + il = ItemLoader(item=i) + il.add_value('name', u'marta') + item = il.load_item() + assert item is i + self.assertEqual(item['summary'], [u'lala']) + self.assertEqual(item['name'], [u'marta']) + + def test_load_item_using_custom_loader(self): + il = TestItemLoader() + il.add_value('name', u'marta') + item = il.load_item() + self.assertEqual(item['name'], [u'Marta']) + + def test_load_item_ignore_none_field_values(self): + def validate_sku(value): + # Let's assume a SKU is only digits. + if value.isdigit(): + return value + + class MyLoader(ItemLoader): + name_out = Compose(lambda vs: vs[0]) # take first which allows empty values + price_out = Compose(TakeFirst(), float) + sku_out = Compose(TakeFirst(), validate_sku) + + valid_fragment = u'SKU: 1234' + invalid_fragment = u'SKU: not available' + sku_re = 'SKU: (.+)' + + il = MyLoader(item={}) + # Should not return "sku: None". + il.add_value('sku', [invalid_fragment], re=sku_re) + # Should not ignore empty values. + il.add_value('name', u'') + il.add_value('price', [u'0']) + self.assertEqual(il.load_item(), { + 'name': u'', + 'price': 0.0, + }) + + il.replace_value('sku', [valid_fragment], re=sku_re) + self.assertEqual(il.load_item()['sku'], u'1234') + + def test_self_referencing_loader(self): + class MyLoader(ItemLoader): + url_out = TakeFirst() + + def img_url_out(self, values): + return (self.get_output_value('url') or '') + values[0] + + il = MyLoader(item={}) + il.add_value('url', 'http://example.com/') + il.add_value('img_url', '1234.png') + self.assertEqual(il.load_item(), { + 'url': 'http://example.com/', + 'img_url': 'http://example.com/1234.png', + }) + + il = MyLoader(item={}) + il.add_value('img_url', '1234.png') + self.assertEqual(il.load_item(), { + 'img_url': '1234.png', + }) + + def test_add_value(self): + il = TestItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_collected_values('name'), [u'Marta']) + self.assertEqual(il.get_output_value('name'), [u'Marta']) + il.add_value('name', u'pepe') + self.assertEqual(il.get_collected_values('name'), [u'Marta', u'Pepe']) + self.assertEqual(il.get_output_value('name'), [u'Marta', u'Pepe']) + + # test add object value + il.add_value('summary', {'key': 1}) + self.assertEqual(il.get_collected_values('summary'), [{'key': 1}]) + + il.add_value(None, u'Jim', lambda x: {'name': x}) + self.assertEqual(il.get_collected_values('name'), [u'Marta', u'Pepe', u'Jim']) + + def test_add_zero(self): + il = NameItemLoader() + il.add_value('name', 0) + self.assertEqual(il.get_collected_values('name'), [0]) + + def test_replace_value(self): + il = TestItemLoader() + il.replace_value('name', u'marta') + self.assertEqual(il.get_collected_values('name'), [u'Marta']) + self.assertEqual(il.get_output_value('name'), [u'Marta']) + il.replace_value('name', u'pepe') + self.assertEqual(il.get_collected_values('name'), [u'Pepe']) + self.assertEqual(il.get_output_value('name'), [u'Pepe']) + + il.replace_value(None, u'Jim', lambda x: {'name': x}) + self.assertEqual(il.get_collected_values('name'), [u'Jim']) + + def test_get_value(self): + il = NameItemLoader() + self.assertEqual(u'FOO', il.get_value([u'foo', u'bar'], TakeFirst(), str.upper)) + self.assertEqual([u'foo', u'bar'], il.get_value([u'name:foo', u'name:bar'], re=u'name:(.*)$')) + self.assertEqual(u'foo', il.get_value([u'name:foo', u'name:bar'], TakeFirst(), re=u'name:(.*)$')) + + il.add_value('name', [u'name:foo', u'name:bar'], TakeFirst(), re=u'name:(.*)$') + self.assertEqual([u'foo'], il.get_collected_values('name')) + il.replace_value('name', u'name:bar', re=u'name:(.*)$') + self.assertEqual([u'bar'], il.get_collected_values('name')) + + def test_iter_on_input_processor_input(self): + class NameFirstItemLoader(NameItemLoader): + name_in = TakeFirst() + + il = NameFirstItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_collected_values('name'), [u'marta']) + il = NameFirstItemLoader() + il.add_value('name', [u'marta', u'jose']) + self.assertEqual(il.get_collected_values('name'), [u'marta']) + + il = NameFirstItemLoader() + il.replace_value('name', u'marta') + self.assertEqual(il.get_collected_values('name'), [u'marta']) + il = NameFirstItemLoader() + il.replace_value('name', [u'marta', u'jose']) + self.assertEqual(il.get_collected_values('name'), [u'marta']) + + il = NameFirstItemLoader() + il.add_value('name', u'marta') + il.add_value('name', [u'jose', u'pedro']) + self.assertEqual(il.get_collected_values('name'), [u'marta', u'jose']) + + def test_map_compose_filter(self): + def filter_world(x): + return None if x == 'world' else x + + proc = MapCompose(filter_world, str.upper) + self.assertEqual(proc(['hello', 'world', 'this', 'is', 'scrapy']), + ['HELLO', 'THIS', 'IS', 'SCRAPY']) + + def test_map_compose_filter_multil(self): + class TestItemLoader(NameItemLoader): + name_in = MapCompose(lambda v: v.title(), lambda v: v[:-1]) + + il = TestItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'Mart']) + item = il.load_item() + self.assertEqual(item['name'], [u'Mart']) + + def test_default_input_processor(self): + il = DefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'mart']) + + def test_inherited_default_input_processor(self): + class InheritDefaultedItemLoader(DefaultedItemLoader): + pass + + il = InheritDefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'mart']) + + def test_input_processor_inheritance(self): + class ChildItemLoader(TestItemLoader): + url_in = MapCompose(lambda v: v.lower()) + + il = ChildItemLoader() + il.add_value('url', u'HTTP://scrapy.ORG') + self.assertEqual(il.get_output_value('url'), [u'http://scrapy.org']) + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'Marta']) + + class ChildChildItemLoader(ChildItemLoader): + url_in = MapCompose(lambda v: v.upper()) + summary_in = MapCompose(lambda v: v) + + il = ChildChildItemLoader() + il.add_value('url', u'http://scrapy.org') + self.assertEqual(il.get_output_value('url'), [u'HTTP://SCRAPY.ORG']) + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'Marta']) + + def test_empty_map_compose(self): + class IdentityDefaultedItemLoader(DefaultedItemLoader): + name_in = MapCompose() + + il = IdentityDefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'marta']) + + def test_identity_input_processor(self): + class IdentityDefaultedItemLoader(DefaultedItemLoader): + name_in = Identity() + + il = IdentityDefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'marta']) + + def test_extend_custom_input_processors(self): + class ChildItemLoader(TestItemLoader): + name_in = MapCompose(TestItemLoader.name_in, str.swapcase) + + il = ChildItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'mARTA']) + + def test_extend_default_input_processors(self): + class ChildDefaultedItemLoader(DefaultedItemLoader): + name_in = MapCompose(DefaultedItemLoader.default_input_processor, str.swapcase) + + il = ChildDefaultedItemLoader() + il.add_value('name', u'marta') + self.assertEqual(il.get_output_value('name'), [u'MART']) + + def test_output_processor_using_function(self): + il = TestItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) + + class TakeFirstItemLoader(TestItemLoader): + name_out = u" ".join + + il = TakeFirstItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), u'Mar Ta') + + def test_output_processor_error(self): + class TestItemLoader(ItemLoader): + default_item_class = TestItem + name_out = MapCompose(float) + + il = TestItemLoader() + il.add_value('name', [u'$10']) + try: + float(u'$10') + except Exception as e: + expected_exc_str = str(e) + + exc = None + try: + il.load_item() + except Exception as e: + exc = e + assert isinstance(exc, ValueError) + s = str(exc) + assert 'name' in s, s + assert '$10' in s, s + assert 'ValueError' in s, s + assert expected_exc_str in s, s + + def test_output_processor_using_classes(self): + il = TestItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) + + class TakeFirstItemLoader(TestItemLoader): + name_out = Join() + + il = TakeFirstItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), u'Mar Ta') + + class TakeFirstItemLoader(TestItemLoader): + name_out = Join("<br>") + + il = TakeFirstItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), u'Mar<br>Ta') + + def test_default_output_processor(self): + il = TestItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) + + class LalaItemLoader(TestItemLoader): + default_output_processor = Identity() + + il = LalaItemLoader() + il.add_value('name', [u'mar', u'ta']) + self.assertEqual(il.get_output_value('name'), [u'Mar', u'Ta']) + + def test_loader_context_on_declaration(self): + class ChildItemLoader(TestItemLoader): + url_in = MapCompose(processor_with_args, key=u'val') + + il = ChildItemLoader() + il.add_value('url', u'text') + self.assertEqual(il.get_output_value('url'), ['val']) + il.replace_value('url', u'text2') + self.assertEqual(il.get_output_value('url'), ['val']) + + def test_loader_context_on_instantiation(self): + class ChildItemLoader(TestItemLoader): + url_in = MapCompose(processor_with_args) + + il = ChildItemLoader(key=u'val') + il.add_value('url', u'text') + self.assertEqual(il.get_output_value('url'), ['val']) + il.replace_value('url', u'text2') + self.assertEqual(il.get_output_value('url'), ['val']) + + def test_loader_context_on_assign(self): + class ChildItemLoader(TestItemLoader): + url_in = MapCompose(processor_with_args) + + il = ChildItemLoader() + il.context['key'] = u'val' + il.add_value('url', u'text') + self.assertEqual(il.get_output_value('url'), ['val']) + il.replace_value('url', u'text2') + self.assertEqual(il.get_output_value('url'), ['val']) + + def test_item_passed_to_input_processor_functions(self): + def processor(value, loader_context): + return loader_context['item']['name'] + + class ChildItemLoader(TestItemLoader): + url_in = MapCompose(processor) + + it = TestItem(name='marta') + il = ChildItemLoader(item=it) + il.add_value('url', u'text') + self.assertEqual(il.get_output_value('url'), ['marta']) + il.replace_value('url', u'text2') + self.assertEqual(il.get_output_value('url'), ['marta']) + + def test_compose_processor(self): + class TestItemLoader(NameItemLoader): + name_out = Compose(lambda v: v[0], lambda v: v.title(), lambda v: v[:-1]) + + il = TestItemLoader() + il.add_value('name', [u'marta', u'other']) + self.assertEqual(il.get_output_value('name'), u'Mart') + item = il.load_item() + self.assertEqual(item['name'], u'Mart') + + def test_partial_processor(self): + def join(values, sep=None, loader_context=None, ignored=None): + if sep is not None: + return sep.join(values) + elif loader_context and 'sep' in loader_context: + return loader_context['sep'].join(values) + else: + return ''.join(values) + + class TestItemLoader(NameItemLoader): + name_out = Compose(partial(join, sep='+')) + url_out = Compose(partial(join, loader_context={'sep': '.'})) + summary_out = Compose(partial(join, ignored='foo')) + + il = TestItemLoader() + il.add_value('name', [u'rabbit', u'hole']) + il.add_value('url', [u'rabbit', u'hole']) + il.add_value('summary', [u'rabbit', u'hole']) + item = il.load_item() + self.assertEqual(item['name'], u'rabbit+hole') + self.assertEqual(item['url'], u'rabbit.hole') + self.assertEqual(item['summary'], u'rabbithole') + + def test_error_input_processor(self): + class TestItem(Item): + name = Field() + + class TestItemLoader(ItemLoader): + default_item_class = TestItem + name_in = MapCompose(float) + + il = TestItemLoader() + self.assertRaises(ValueError, il.add_value, 'name', + [u'marta', u'other']) + + def test_error_output_processor(self): + class TestItem(Item): + name = Field() + + class TestItemLoader(ItemLoader): + default_item_class = TestItem + name_out = Compose(Join(), float) + + il = TestItemLoader() + il.add_value('name', u'marta') + with self.assertRaises(ValueError): + il.load_item() + + def test_error_processor_as_argument(self): + class TestItem(Item): + name = Field() + + class TestItemLoader(ItemLoader): + default_item_class = TestItem + + il = TestItemLoader() + self.assertRaises(ValueError, il.add_value, 'name', + [u'marta', u'other'], Compose(float)) + + +class InitializationFromDictTest(unittest.TestCase): + + item_class = dict + + def test_keep_single_value(self): + """Loaded item should contain values from the initial item""" + input_item = self.item_class(name='foo') + il = ItemLoader(item=input_item) + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(dict(loaded_item), {'name': ['foo']}) + + def test_keep_list(self): + """Loaded item should contain values from the initial item""" + input_item = self.item_class(name=['foo', 'bar']) + il = ItemLoader(item=input_item) + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(dict(loaded_item), {'name': ['foo', 'bar']}) + + def test_add_value_singlevalue_singlevalue(self): + """Values added after initialization should be appended""" + input_item = self.item_class(name='foo') + il = ItemLoader(item=input_item) + il.add_value('name', 'bar') + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(dict(loaded_item), {'name': ['foo', 'bar']}) + + def test_add_value_singlevalue_list(self): + """Values added after initialization should be appended""" + input_item = self.item_class(name='foo') + il = ItemLoader(item=input_item) + il.add_value('name', ['item', 'loader']) + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(dict(loaded_item), {'name': ['foo', 'item', 'loader']}) + + def test_add_value_list_singlevalue(self): + """Values added after initialization should be appended""" + input_item = self.item_class(name=['foo', 'bar']) + il = ItemLoader(item=input_item) + il.add_value('name', 'qwerty') + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(dict(loaded_item), {'name': ['foo', 'bar', 'qwerty']}) + + def test_add_value_list_list(self): + """Values added after initialization should be appended""" + input_item = self.item_class(name=['foo', 'bar']) + il = ItemLoader(item=input_item) + il.add_value('name', ['item', 'loader']) + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(dict(loaded_item), {'name': ['foo', 'bar', 'item', 'loader']}) + + def test_get_output_value_singlevalue(self): + """Getting output value must not remove value from item""" + input_item = self.item_class(name='foo') + il = ItemLoader(item=input_item) + self.assertEqual(il.get_output_value('name'), ['foo']) + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(loaded_item, dict({'name': ['foo']})) + + def test_get_output_value_list(self): + """Getting output value must not remove value from item""" + input_item = self.item_class(name=['foo', 'bar']) + il = ItemLoader(item=input_item) + self.assertEqual(il.get_output_value('name'), ['foo', 'bar']) + loaded_item = il.load_item() + self.assertIsInstance(loaded_item, self.item_class) + self.assertEqual(loaded_item, dict({'name': ['foo', 'bar']})) + + def test_values_single(self): + """Values from initial item must be added to loader._values""" + input_item = self.item_class(name='foo') + il = ItemLoader(item=input_item) + self.assertEqual(il._values.get('name'), ['foo']) + + def test_values_list(self): + """Values from initial item must be added to loader._values""" + input_item = self.item_class(name=['foo', 'bar']) + il = ItemLoader(item=input_item) + self.assertEqual(il._values.get('name'), ['foo', 'bar']) + + +class BaseNoInputReprocessingLoader(ItemLoader): + title_in = MapCompose(str.upper) + title_out = TakeFirst() + + +class NoInputReprocessingDictLoader(BaseNoInputReprocessingLoader): + default_item_class = dict + + +class NoInputReprocessingFromDictTest(unittest.TestCase): + """ + Loaders initialized from loaded items must not reprocess fields (dict instances) + """ + def test_avoid_reprocessing_with_initial_values_single(self): + il = NoInputReprocessingDictLoader(item=dict(title='foo')) + il_loaded = il.load_item() + self.assertEqual(il_loaded, dict(title='foo')) + self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='foo')) + + def test_avoid_reprocessing_with_initial_values_list(self): + il = NoInputReprocessingDictLoader(item=dict(title=['foo', 'bar'])) + il_loaded = il.load_item() + self.assertEqual(il_loaded, dict(title='foo')) + self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='foo')) + + def test_avoid_reprocessing_without_initial_values_single(self): + il = NoInputReprocessingDictLoader() + il.add_value('title', 'foo') + il_loaded = il.load_item() + self.assertEqual(il_loaded, dict(title='FOO')) + self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='FOO')) + + def test_avoid_reprocessing_without_initial_values_list(self): + il = NoInputReprocessingDictLoader() + il.add_value('title', ['foo', 'bar']) + il_loaded = il.load_item() + self.assertEqual(il_loaded, dict(title='FOO')) + self.assertEqual(NoInputReprocessingDictLoader(item=il_loaded).load_item(), dict(title='FOO')) + + +class TestOutputProcessorDict(unittest.TestCase): + def test_output_processor(self): + + class TempDict(dict): + def __init__(self, *args, **kwargs): + super(TempDict, self).__init__(self, *args, **kwargs) + self.setdefault('temp', 0.3) + + class TempLoader(ItemLoader): + default_item_class = TempDict + default_input_processor = Identity() + default_output_processor = Compose(TakeFirst()) + + loader = TempLoader() + item = loader.load_item() + self.assertIsInstance(item, TempDict) + self.assertEqual(dict(item), {'temp': 0.3}) + + +class ProcessorsTest(unittest.TestCase): + + def test_take_first(self): + proc = TakeFirst() + self.assertEqual(proc([None, '', 'hello', 'world']), 'hello') + self.assertEqual(proc([None, '', 0, 'hello', 'world']), 0) + + def test_identity(self): + proc = Identity() + self.assertEqual(proc([None, '', 'hello', 'world']), + [None, '', 'hello', 'world']) + + def test_join(self): + proc = Join() + self.assertRaises(TypeError, proc, [None, '', 'hello', 'world']) + self.assertEqual(proc(['', 'hello', 'world']), u' hello world') + self.assertEqual(proc(['hello', 'world']), u'hello world') + self.assertIsInstance(proc(['hello', 'world']), str) + + def test_compose(self): + proc = Compose(lambda v: v[0], str.upper) + self.assertEqual(proc(['hello', 'world']), 'HELLO') + proc = Compose(str.upper) + self.assertEqual(proc(None), None) + proc = Compose(str.upper, stop_on_none=False) + self.assertRaises(ValueError, proc, None) + proc = Compose(str.upper, lambda x: x + 1) + self.assertRaises(ValueError, proc, 'hello') + + def test_mapcompose(self): + def filter_world(x): + return None if x == 'world' else x + proc = MapCompose(filter_world, str.upper) + self.assertEqual(proc([u'hello', u'world', u'this', u'is', u'scrapy']), + [u'HELLO', u'THIS', u'IS', u'SCRAPY']) + proc = MapCompose(filter_world, str.upper) + self.assertEqual(proc(None), []) + proc = MapCompose(filter_world, str.upper) + self.assertRaises(ValueError, proc, [1]) + proc = MapCompose(filter_world, lambda x: x + 1) + self.assertRaises(ValueError, proc, 'hello') + + +class SelectJmesTestCase(unittest.TestCase): + test_list_equals = { + 'simple': ('foo.bar', {"foo": {"bar": "baz"}}, "baz"), + 'invalid': ('foo.bar.baz', {"foo": {"bar": "baz"}}, None), + 'top_level': ('foo', {"foo": {"bar": "baz"}}, {"bar": "baz"}), + 'double_vs_single_quote_string': ('foo.bar', {"foo": {"bar": "baz"}}, "baz"), + 'dict': ( + 'foo.bar[*].name', + {"foo": {"bar": [{"name": "one"}, {"name": "two"}]}}, + ['one', 'two'] + ), + 'list': ('[1]', [1, 2], 2) + } + + def test_output(self): + for tl in self.test_list_equals: + expr, test_list, expected = self.test_list_equals[tl] + test = SelectJmes(expr)(test_list) + self.assertEqual( + test, + expected, + msg='test "{}" got {} expected {}'.format(tl, test, expected) + ) + + +# Functions as processors + +def function_processor_strip(iterable): + return [x.strip() for x in iterable] + + +def function_processor_upper(iterable): + return [x.upper() for x in iterable] + + +class FunctionProcessorItem(Item): + foo = Field( + input_processor=function_processor_strip, + output_processor=function_processor_upper, + ) + + +class FunctionProcessorDictLoader(ItemLoader): + default_item_class = dict + foo_in = function_processor_strip + foo_out = function_processor_upper + + +class FunctionProcessorTestCase(unittest.TestCase): + + def test_processor_defined_in_item_loader(self): + lo = FunctionProcessorDictLoader() + lo.add_value('foo', ' bar ') + lo.add_value('foo', [' asdf ', ' qwerty ']) + self.assertEqual( + dict(lo.load_item()), + {'foo': ['BAR', 'ASDF', 'QWERTY']} + ) + + +class DeprecatedUtilityFunctionsTestCase(unittest.TestCase): + + def test_deprecated_wrap_loader_context(self): + def function(*args): + return None + + with warnings.catch_warnings(record=True) as w: + wrap_loader_context(function, context=dict()) + + assert len(w) == 1 + assert issubclass(w[0].category, ScrapyDeprecationWarning) + + def test_deprecated_extract_regex(self): + with warnings.catch_warnings(record=True) as w: + extract_regex(r'\w+', 'this is a test') + + assert len(w) == 1 + assert issubclass(w[0].category, ScrapyDeprecationWarning) + + +if __name__ == "__main__": + unittest.main() From 0e0d1ad64323033017c4a01893b5337a980cb5ec Mon Sep 17 00:00:00 2001 From: Marc <noviluni@gmail.com> Date: Thu, 16 Jul 2020 14:19:46 +0200 Subject: [PATCH 51/56] remove python 2 reminiscence in cookies --- scrapy/http/cookies.py | 3 --- tests/test_http_cookies.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py index 3e810992c..0c97e6999 100644 --- a/scrapy/http/cookies.py +++ b/scrapy/http/cookies.py @@ -186,9 +186,6 @@ class WrappedResponse: def info(self): return self - # python3 cookiejars calls get_all def get_all(self, name, default=None): return [to_unicode(v, errors='replace') for v in self.response.headers.getlist(name)] - # python2 cookiejars calls getheaders - getheaders = get_all diff --git a/tests/test_http_cookies.py b/tests/test_http_cookies.py index 45ddb42ba..540e27907 100644 --- a/tests/test_http_cookies.py +++ b/tests/test_http_cookies.py @@ -64,9 +64,6 @@ class WrappedResponseTest(TestCase): def test_info(self): self.assertIs(self.wrapped.info(), self.wrapped) - def test_getheaders(self): - self.assertEqual(self.wrapped.getheaders('content-type'), ['text/html']) - def test_get_all(self): # get_all result must be native string self.assertEqual(self.wrapped.get_all('content-type'), ['text/html']) From b97a39fda0b72d8a4dcd631599e5d9bf530a5bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Hern=C3=A1ndez?= <noviluni@gmail.com> Date: Thu, 16 Jul 2020 17:38:22 +0200 Subject: [PATCH 52/56] deprecate retry_on_eintr (#4683) --- scrapy/utils/python.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index afa8a8135..9204977cf 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -285,6 +285,7 @@ class WeakKeyCache: return self._weakdict[key] +@deprecated def retry_on_eintr(function, *args, **kw): """Run a function and retry it while getting EINTR errors""" while True: From d29bec60d795b13ecb6e5978cb9e4d8fbd298b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= <adrian@chaves.io> Date: Thu, 16 Jul 2020 23:19:24 +0200 Subject: [PATCH 53/56] Upgrade PyPy for CI, and test both 3.5 (oldest) and 3.6 (newest) (#4504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Upgrade PyPy for CI, and test both 3.5 (oldest) and 3.6 (newest) * Log a detailed error message to discover why MockServer is not working * Go for all lines! * Disable tests based on mitmproxy while running on PyPy * Fix test_get_func_args for PyPy 3.6+ * Make testPayloadDefaultCiphers work regardless of OpenSSL default ciphers * Crossing fingers… * Rename: testPayloadDefaultCiphers → testPayloadDisabledCipher * Test the PyPy version currently documented as the minimum required version * Fix the PYPY_VERSION tag * Update the documentation about supported PyPy versions * Also test the latest 3.5 Python version with PyPy * Fix the PYPY_VERSION value for the latest 3.5 version * Use pinned dependencies for asyncio and PyPy tests against oldest supported Python versions * Fix PyPy installation for the pypy3-pinned Tox environment * Try installing Cython * Maybe PyPy requires lxml 3.6.0? * install.rst: minor clarification * lxml 4.0.0 is required on PyPy * Require setuptools 18.5+ * Revert "Require setuptools 18.5+" This reverts commit 017ec33ac2d237523cdd53be9be8169dd540759e. * Maintain lxml as a dependency if setuptools < 18.5 is used --- .travis.yml | 16 +++++++++----- docs/faq.rst | 14 ------------ docs/intro/install.rst | 12 +++++++--- setup.py | 44 +++++++++++++++++++++++-------------- tests/test_proxy_connect.py | 2 ++ tests/test_utils_python.py | 6 ++++- tests/test_webclient.py | 6 +++-- tox.ini | 31 +++++++++++++++++--------- 8 files changed, 80 insertions(+), 51 deletions(-) diff --git a/.travis.yml b/.travis.yml index b403ac54c..db720b918 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,19 +18,25 @@ matrix: - env: TOXENV=typing python: 3.8 - - env: TOXENV=pypy3 - env: TOXENV=pinned python: 3.5.2 - - env: TOXENV=asyncio + - env: TOXENV=asyncio-pinned python: 3.5.2 # We use additional code to support 3.5.3 and earlier + - env: TOXENV=pypy3-pinned PYPY_VERSION=3-v5.9.0 + - env: TOXENV=py python: 3.5 - env: TOXENV=asyncio python: 3.5 # We use specific code to support >= 3.5.4, < 3.6 + - env: TOXENV=pypy3 PYPY_VERSION=3.5-v7.0.0 + - env: TOXENV=py python: 3.6 + - env: TOXENV=pypy3 PYPY_VERSION=3.6-v7.3.1 + - env: TOXENV=py python: 3.7 + - env: TOXENV=py PYPI_RELEASE_JOB=true python: 3.8 dist: bionic @@ -42,9 +48,9 @@ matrix: dist: bionic install: - | - if [ "$TOXENV" = "pypy3" ]; then - export PYPY_VERSION="pypy3.5-5.9-beta-linux_x86_64-portable" - wget "https://bitbucket.org/squeaky/portable-pypy/downloads/${PYPY_VERSION}.tar.bz2" + if [[ ! -z "$PYPY_VERSION" ]]; then + export PYPY_VERSION="pypy$PYPY_VERSION-linux64" + wget "https://bitbucket.org/pypy/pypy/downloads/${PYPY_VERSION}.tar.bz2" tar -jxf ${PYPY_VERSION}.tar.bz2 virtualenv --python="$PYPY_VERSION/bin/pypy3" "$HOME/virtualenvs/$PYPY_VERSION" source "$HOME/virtualenvs/$PYPY_VERSION/bin/activate" diff --git a/docs/faq.rst b/docs/faq.rst index d5ea3cb87..ea2c8216f 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -64,20 +64,6 @@ Here's an example spider using BeautifulSoup API, with ``lxml`` as the HTML pars .. _BeautifulSoup's official documentation: https://www.crummy.com/software/BeautifulSoup/bs4/doc/#specifying-the-parser-to-use -.. _faq-python-versions: - -What Python versions does Scrapy support? ------------------------------------------ - -Scrapy is supported under Python 3.5.2+ -under CPython (default Python implementation) and PyPy (starting with PyPy 5.9). -Python 3 support was added in Scrapy 1.1. -PyPy support was added in Scrapy 1.4, PyPy3 support was added in Scrapy 1.5. -Python 2 support was dropped in Scrapy 2.0. - -.. note:: - For Python 3 support on Windows, it is recommended to use - Anaconda/Miniconda as :ref:`outlined in the installation guide <intro-install-windows>`. Did Scrapy "steal" X from Django? --------------------------------- diff --git a/docs/intro/install.rst b/docs/intro/install.rst index fb64d443c..6d65ae2ee 100644 --- a/docs/intro/install.rst +++ b/docs/intro/install.rst @@ -4,12 +4,18 @@ Installation guide ================== +.. _faq-python-versions: + +Supported Python versions +========================= + +Scrapy requires Python 3.5.2+, either the CPython implementation (default) or +the PyPy 5.9+ implementation (see :ref:`python:implementations`). + + Installing Scrapy ================= -Scrapy runs on Python 3.5.2 or above under CPython (default Python -implementation) and PyPy (starting with PyPy 5.9). - If you're using `Anaconda`_ or `Miniconda`_, you can install the package from the `conda-forge`_ channel, which has up-to-date packages for Linux, Windows and macOS. diff --git a/setup.py b/setup.py index f8d9b491b..58090f7a2 100644 --- a/setup.py +++ b/setup.py @@ -18,12 +18,39 @@ def has_environment_marker_platform_impl_support(): return parse_version(setuptools_version) >= parse_version('18.5') +install_requires = [ + 'Twisted>=17.9.0', + 'cryptography>=2.0', + 'cssselect>=0.9.1', + 'itemloaders>=1.0.1', + 'lxml>=3.5.0', + 'parsel>=1.5.0', + 'PyDispatcher>=2.0.5', + 'pyOpenSSL>=16.2.0', + 'queuelib>=1.4.2', + 'service_identity>=16.0.0', + 'w3lib>=1.17.0', + 'zope.interface>=4.1.3', + 'protego>=0.1.15', + 'itemadapter>=0.1.0', +] extras_require = {} if has_environment_marker_platform_impl_support(): + extras_require[':platform_python_implementation == "CPython"'] = [ + 'lxml>=3.5.0', + ] extras_require[':platform_python_implementation == "PyPy"'] = [ + # Earlier lxml versions are affected by + # https://bitbucket.org/pypy/pypy/issues/2498/cython-on-pypy-3-dict-object-has-no, + # which was fixed in Cython 0.26, released on 2017-06-19, and used to + # generate the C headers of lxml release tarballs published since then, the + # first of which was: + 'lxml>=4.0.0', 'PyPyDispatcher>=2.1.0', ] +else: + install_requires.append('lxml>=3.5.0') setup( @@ -67,21 +94,6 @@ setup( 'Topic :: Software Development :: Libraries :: Python Modules', ], python_requires='>=3.5.2', - install_requires=[ - 'Twisted>=17.9.0', - 'cryptography>=2.0', - 'cssselect>=0.9.1', - 'itemloaders>=1.0.1', - 'lxml>=3.5.0', - 'parsel>=1.5.0', - 'PyDispatcher>=2.0.5', - 'pyOpenSSL>=16.2.0', - 'queuelib>=1.4.2', - 'service_identity>=16.0.0', - 'w3lib>=1.17.0', - 'zope.interface>=4.1.3', - 'protego>=0.1.15', - 'itemadapter>=0.1.0', - ], + install_requires=install_requires, extras_require=extras_require, ) diff --git a/tests/test_proxy_connect.py b/tests/test_proxy_connect.py index fc5658ae7..a56e3c39a 100644 --- a/tests/test_proxy_connect.py +++ b/tests/test_proxy_connect.py @@ -60,6 +60,8 @@ def _wrong_credentials(proxy_url): @skipIf(sys.version_info < (3, 5, 4), "requires mitmproxy < 3.0.0, which these tests do not support") +@skipIf("pypy" in sys.executable, + "mitmproxy does not support PyPy") @skipIf(platform.system() == 'Windows' and sys.version_info < (3, 7), "mitmproxy does not support Windows when running Python < 3.7") class ProxyConnectTestCase(TestCase): diff --git a/tests/test_utils_python.py b/tests/test_utils_python.py index 65e6ba876..ebce3c079 100644 --- a/tests/test_utils_python.py +++ b/tests/test_utils_python.py @@ -4,6 +4,7 @@ import operator import platform import unittest from itertools import count +from sys import version_info from warnings import catch_warnings from scrapy.utils.python import ( @@ -214,9 +215,12 @@ class UtilsPythonTestCase(unittest.TestCase): else: self.assertEqual( get_func_args(str.split, stripself=True), ['sep', 'maxsplit']) - self.assertEqual(get_func_args(" ".join, stripself=True), ['list']) self.assertEqual( get_func_args(operator.itemgetter(2), stripself=True), ['obj']) + if version_info < (3, 6): + self.assertEqual(get_func_args(" ".join, stripself=True), ['list']) + else: + self.assertEqual(get_func_args(" ".join, stripself=True), ['iterable']) def test_without_none_values(self): self.assertEqual(without_none_values([1, None, 3, 4]), [1, 3, 4]) diff --git a/tests/test_webclient.py b/tests/test_webclient.py index c1c5945c2..ee64d455c 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -413,7 +413,9 @@ class WebClientCustomCiphersSSLTestCase(WebClientSSLTestCase): self.getURL("payload"), body=s, contextFactory=client_context_factory ).addCallback(self.assertEqual, to_bytes(s)) - def testPayloadDefaultCiphers(self): + def testPayloadDisabledCipher(self): s = "0123456789" * 10 - d = getPage(self.getURL("payload"), body=s, contextFactory=ScrapyClientContextFactory()) + settings = Settings({'DOWNLOADER_CLIENT_TLS_CIPHERS': 'ECDHE-RSA-AES256-GCM-SHA384'}) + client_context_factory = create_instance(ScrapyClientContextFactory, settings=settings, crawler=None) + d = getPage(self.getURL("payload"), body=s, contextFactory=client_context_factory) return self.assertFailure(d, OpenSSL.SSL.Error) diff --git a/tox.ini b/tox.ini index 5d79739bb..4557c63e3 100644 --- a/tox.ini +++ b/tox.ini @@ -58,11 +58,6 @@ deps = commands = pylint conftest.py docs extras scrapy setup.py tests -[testenv:pypy3] -basepython = pypy3 -commands = - py.test {posargs:--durations=10 docs scrapy tests} - [pinned] deps = -ctests/constraints.txt @@ -85,7 +80,6 @@ deps = Pillow==3.4.2 [testenv:pinned] -basepython = python3 deps = {[pinned]deps} lxml==3.5.0 @@ -104,6 +98,27 @@ deps = reppy robotexclusionrulesparser +[testenv:asyncio] +commands = + {[testenv]commands} --reactor=asyncio + +[testenv:asyncio-pinned] +commands = {[testenv:asyncio]commands} +deps = {[testenv:pinned]deps} + +[testenv:pypy3] +basepython = pypy3 +commands = + py.test {posargs:--durations=10 docs scrapy tests} + +[testenv:pypy3-pinned] +basepython = {[testenv:pypy3]basepython} +commands = {[testenv:pypy3]commands} +deps = + {[pinned]deps} + lxml==4.0.0 + PyPyDispatcher==2.1.0 + [docs] changedir = docs deps = @@ -135,7 +150,3 @@ deps = {[docs]deps} setenv = {[docs]setenv} commands = sphinx-build -W -b linkcheck . {envtmpdir}/linkcheck - -[testenv:asyncio] -commands = - {[testenv]commands} --reactor=asyncio From 62a4ede5e995f83abd5a90f7dd6ac242f2f3870d Mon Sep 17 00:00:00 2001 From: Artur Shellunts <shellunts.artur@gmail.com> Date: Fri, 17 Jul 2020 12:40:04 +0200 Subject: [PATCH 54/56] Remove deprecated classes BaseSgmlLinkExtractor, RegexLinkExtractor and SgmlLinkExtractor (#4356) --- scrapy/linkextractors/regex.py | 41 ----- scrapy/linkextractors/sgml.py | 151 ------------------ tests/ignores.txt | 2 - .../link_extractor/linkextractor.html | 2 +- .../link_extractor/linkextractor_latin1.html | 2 +- 5 files changed, 2 insertions(+), 196 deletions(-) delete mode 100644 scrapy/linkextractors/regex.py delete mode 100644 scrapy/linkextractors/sgml.py diff --git a/scrapy/linkextractors/regex.py b/scrapy/linkextractors/regex.py deleted file mode 100644 index 3f2557248..000000000 --- a/scrapy/linkextractors/regex.py +++ /dev/null @@ -1,41 +0,0 @@ -import re -from urllib.parse import urljoin - -from w3lib.html import remove_tags, replace_entities, replace_escape_chars, get_base_url - -from scrapy.link import Link -from scrapy.linkextractors.sgml import SgmlLinkExtractor - - -linkre = re.compile( - "<a\s.*?href=(\"[.#]+?\"|\'[.#]+?\'|[^\s]+?)(>|\s.*?>)(.*?)<[/ ]?a>", - re.DOTALL | re.IGNORECASE) - - -def clean_link(link_text): - """Remove leading and trailing whitespace and punctuation""" - return link_text.strip("\t\r\n '\"\x0c") - - -class RegexLinkExtractor(SgmlLinkExtractor): - """High performant link extractor""" - - def _extract_links(self, response_text, response_url, response_encoding, base_url=None): - def clean_text(text): - return replace_escape_chars(remove_tags(text.decode(response_encoding))).strip() - - def clean_url(url): - clean_url = '' - try: - clean_url = urljoin(base_url, replace_entities(clean_link(url.decode(response_encoding)))) - except ValueError: - pass - return clean_url - - if base_url is None: - base_url = get_base_url(response_text, response_url, response_encoding) - - links_text = linkre.findall(response_text) - return [Link(clean_url(url).encode(response_encoding), - clean_text(text)) - for url, _, text in links_text] diff --git a/scrapy/linkextractors/sgml.py b/scrapy/linkextractors/sgml.py deleted file mode 100644 index 2ba6bca45..000000000 --- a/scrapy/linkextractors/sgml.py +++ /dev/null @@ -1,151 +0,0 @@ -""" -SGMLParser-based Link extractors -""" -import warnings -from urllib.parse import urljoin -from sgmllib import SGMLParser - -from w3lib.url import safe_url_string, canonicalize_url -from w3lib.html import strip_html5_whitespace - -from scrapy.link import Link -from scrapy.linkextractors import FilteringLinkExtractor -from scrapy.utils.misc import arg_to_iter, rel_has_nofollow -from scrapy.utils.python import unique as unique_list, to_unicode -from scrapy.utils.response import get_base_url -from scrapy.exceptions import ScrapyDeprecationWarning - - -class BaseSgmlLinkExtractor(SGMLParser): - - def __init__(self, tag="a", attr="href", unique=False, process_value=None, - strip=True, canonicalized=False): - warnings.warn( - "BaseSgmlLinkExtractor is deprecated and will be removed in future releases. " - "Please use scrapy.linkextractors.LinkExtractor", - ScrapyDeprecationWarning, stacklevel=2, - ) - SGMLParser.__init__(self) - self.scan_tag = tag if callable(tag) else lambda t: t == tag - self.scan_attr = attr if callable(attr) else lambda a: a == attr - self.process_value = (lambda v: v) if process_value is None else process_value - self.current_link = None - self.unique = unique - self.strip = strip - if canonicalized: - self.link_key = lambda link: link.url - else: - self.link_key = lambda link: canonicalize_url(link.url, - keep_fragments=True) - - def _extract_links(self, response_text, response_url, response_encoding, base_url=None): - """ Do the real extraction work """ - self.reset() - self.feed(response_text) - self.close() - - ret = [] - if base_url is None: - base_url = urljoin(response_url, self.base_url) if self.base_url else response_url - for link in self.links: - if isinstance(link.url, str): - link.url = link.url.encode(response_encoding) - try: - link.url = urljoin(base_url, link.url) - except ValueError: - continue - link.url = safe_url_string(link.url, response_encoding) - link.text = to_unicode(link.text, response_encoding, errors='replace').strip() - ret.append(link) - - return ret - - def _process_links(self, links): - """ Normalize and filter extracted links - - The subclass should override it if necessary - """ - return unique_list(links, key=self.link_key) if self.unique else links - - def extract_links(self, response): - # wrapper needed to allow to work directly with text - links = self._extract_links(response.body, response.url, response.encoding) - links = self._process_links(links) - return links - - def reset(self): - SGMLParser.reset(self) - self.links = [] - self.base_url = None - self.current_link = None - - def unknown_starttag(self, tag, attrs): - if tag == 'base': - self.base_url = dict(attrs).get('href') - if self.scan_tag(tag): - for attr, value in attrs: - if self.scan_attr(attr): - if self.strip and value is not None: - value = strip_html5_whitespace(value) - url = self.process_value(value) - if url is not None: - link = Link(url=url, nofollow=rel_has_nofollow(dict(attrs).get('rel'))) - self.links.append(link) - self.current_link = link - - def unknown_endtag(self, tag): - if self.scan_tag(tag): - self.current_link = None - - def handle_data(self, data): - if self.current_link: - self.current_link.text = self.current_link.text + data - - def matches(self, url): - """This extractor matches with any url, since - it doesn't contain any patterns""" - return True - - -class SgmlLinkExtractor(FilteringLinkExtractor): - - def __init__(self, allow=(), deny=(), allow_domains=(), deny_domains=(), restrict_xpaths=(), - tags=('a', 'area'), attrs=('href',), canonicalize=False, unique=True, - process_value=None, deny_extensions=None, restrict_css=(), - strip=True, restrict_text=()): - warnings.warn( - "SgmlLinkExtractor is deprecated and will be removed in future releases. " - "Please use scrapy.linkextractors.LinkExtractor", - ScrapyDeprecationWarning, stacklevel=2, - ) - - tags, attrs = set(arg_to_iter(tags)), set(arg_to_iter(attrs)) - tag_func = lambda x: x in tags - attr_func = lambda x: x in attrs - - with warnings.catch_warnings(): - warnings.simplefilter('ignore', ScrapyDeprecationWarning) - lx = BaseSgmlLinkExtractor(tag=tag_func, attr=attr_func, - unique=unique, process_value=process_value, strip=strip, - canonicalized=canonicalize) - - super(SgmlLinkExtractor, self).__init__(lx, allow=allow, deny=deny, - allow_domains=allow_domains, deny_domains=deny_domains, - restrict_xpaths=restrict_xpaths, restrict_css=restrict_css, - canonicalize=canonicalize, deny_extensions=deny_extensions, - restrict_text=restrict_text) - - def extract_links(self, response): - base_url = None - if self.restrict_xpaths: - base_url = get_base_url(response) - body = u''.join(f - for x in self.restrict_xpaths - for f in response.xpath(x).getall() - ).encode(response.encoding, errors='xmlcharrefreplace') - else: - body = response.body - - links = self._extract_links(body, response.url, response.encoding, base_url) - links = self._process_links(links) - return links diff --git a/tests/ignores.txt b/tests/ignores.txt index f6e0d6fbe..222288841 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -1,5 +1,3 @@ -scrapy/linkextractors/sgml.py -scrapy/linkextractors/regex.py scrapy/downloadermiddlewares/cookies.py scrapy/extensions/statsmailer.py scrapy/extensions/memusage.py diff --git a/tests/sample_data/link_extractor/linkextractor.html b/tests/sample_data/link_extractor/linkextractor.html index 7d5db368a..2307ea865 100644 --- a/tests/sample_data/link_extractor/linkextractor.html +++ b/tests/sample_data/link_extractor/linkextractor.html @@ -1,7 +1,7 @@ <html> <head> <base href='http://example.com' /> -<title>Sample page with links for testing RegexLinkExtractor +Sample page with links for testing LinkExtractor
diff --git a/tests/sample_data/link_extractor/linkextractor_latin1.html b/tests/sample_data/link_extractor/linkextractor_latin1.html index fc31d7e5d..e7eee18de 100644 --- a/tests/sample_data/link_extractor/linkextractor_latin1.html +++ b/tests/sample_data/link_extractor/linkextractor_latin1.html @@ -2,7 +2,7 @@ - Sample page with links for testing RegexLinkExtractor + Sample page with links for testing LinkExtractor
From de297a3a167a6be3ac1ed94a891effefc12a6d00 Mon Sep 17 00:00:00 2001 From: Akshay Sharma <42249933+AKSHAYSHARMAJS@users.noreply.github.com> Date: Mon, 20 Jul 2020 17:53:38 +0530 Subject: [PATCH 55/56] enable ANSI color (instead of ANSI color codes) in the Windows terminal #4393 (#4403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * changed ie. -> i.e.(spelling error) on lines 667, 763 (issue scrapy#4332) * updated all text files for issue #4332 (ie. -> i.e.) * Apply ie. → i.e. in source comments * ie → e.g. * modified scrapy/utils/display.py to stop ANSI color sequences in the Windows terminal (issue #4393) * modified scrapy/utils/display.py to stop ANSI color sequences in the Windows terminal (issue #4393) * enabled virtual terminal processing (pr #4403) * check for specific windows 10 version (pr #4403) * fixing flake-8 test (pr #4403) * added error handling for terminal info (pr #4403) * corrected stderr (pr #4403) * changed orientation, removed unwanted spaces (pr #4403) * no need for style variable (pr #4403) * fixing trailing whitespaces * commenting windows check * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * small fixes * Shifting _color_support_info() function * enabled virtual terminal processing (pr #4403) * check for specific windows 10 version (pr #4403) * fixing flake-8 test (pr #4403) * added error handling for terminal info (pr #4403) * corrected stderr (pr #4403) * changed orientation, removed unwanted spaces (pr #4403) * no need for style variable (pr #4403) * fixing trailing whitespaces * commenting windows check * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * Update scrapy/utils/display.py Co-Authored-By: Adrián Chaves * small fixes * Shifting _color_support_info() function * error handling * error handlingy * raise ValueError * added in-built function for version comparison * recommit changes * changed check -> parse * version comparison -> parse_version * added scrapy/utils/display.py in pytest.ini * Trigger * Add simple test for scrapy.utils.display._colorize * Flake8: E501 for tests/test_utils_display.py * assertEquals -> assertEqual * Normal formatter for all platforms * separate test for windows * all curses under try block * added global TestStr * more test added * small fix * covering exceptions * windows test failing * Refactor output color handling * Fix pprint test * fix flake8 Co-authored-by: Adrián Chaves Co-authored-by: Eugenio Lacuesta --- scrapy/utils/display.py | 28 +++++++++++-- tests/test_utils_display.py | 78 +++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 tests/test_utils_display.py diff --git a/scrapy/utils/display.py b/scrapy/utils/display.py index 9735220ef..f4d17224b 100644 --- a/scrapy/utils/display.py +++ b/scrapy/utils/display.py @@ -2,20 +2,42 @@ pprint and pformat wrappers with colorization support """ +import ctypes +import platform import sys +from distutils.version import LooseVersion as parse_version from pprint import pformat as pformat_ +def _enable_windows_terminal_processing(): + # https://stackoverflow.com/a/36760881 + kernel32 = ctypes.windll.kernel32 + return bool(kernel32.SetConsoleMode(kernel32.GetStdHandle(-11), 7)) + + +def _tty_supports_color(): + if sys.platform != "win32": + return True + + if parse_version(platform.version()) < parse_version("10.0.14393"): + return True + + # Windows >= 10.0.14393 interprets ANSI escape sequences providing terminal + # processing is enabled. + return _enable_windows_terminal_processing() + + def _colorize(text, colorize=True): - if not colorize or not sys.stdout.isatty(): + if not colorize or not sys.stdout.isatty() or not _tty_supports_color(): return text try: from pygments import highlight + except ImportError: + return text + else: from pygments.formatters import TerminalFormatter from pygments.lexers import PythonLexer return highlight(text, PythonLexer(), TerminalFormatter()) - except ImportError: - return text def pformat(obj, *args, **kwargs): diff --git a/tests/test_utils_display.py b/tests/test_utils_display.py new file mode 100644 index 000000000..9ec8311d9 --- /dev/null +++ b/tests/test_utils_display.py @@ -0,0 +1,78 @@ +from io import StringIO + +from unittest import mock, TestCase + +from scrapy.utils.display import pformat, pprint + + +class TestDisplay(TestCase): + object = {'a': 1} + colorized_string = ( + "{\x1b[33m'\x1b[39;49;00m\x1b[33ma\x1b[39;49;00m\x1b[33m'" + "\x1b[39;49;00m: \x1b[34m1\x1b[39;49;00m}\n" + ) + plain_string = "{'a': 1}" + + @mock.patch('sys.platform', 'linux') + @mock.patch("sys.stdout.isatty") + def test_pformat(self, isatty): + isatty.return_value = True + self.assertEqual(pformat(self.object), self.colorized_string) + + @mock.patch("sys.stdout.isatty") + def test_pformat_dont_colorize(self, isatty): + isatty.return_value = True + self.assertEqual(pformat(self.object, colorize=False), self.plain_string) + + def test_pformat_not_tty(self): + self.assertEqual(pformat(self.object), self.plain_string) + + @mock.patch('sys.platform', 'win32') + @mock.patch('platform.version') + @mock.patch("sys.stdout.isatty") + def test_pformat_old_windows(self, isatty, version): + isatty.return_value = True + version.return_value = '10.0.14392' + self.assertEqual(pformat(self.object), self.colorized_string) + + @mock.patch('sys.platform', 'win32') + @mock.patch('scrapy.utils.display._enable_windows_terminal_processing') + @mock.patch('platform.version') + @mock.patch("sys.stdout.isatty") + def test_pformat_windows_no_terminal_processing(self, isatty, version, terminal_processing): + isatty.return_value = True + version.return_value = '10.0.14393' + terminal_processing.return_value = False + self.assertEqual(pformat(self.object), self.plain_string) + + @mock.patch('sys.platform', 'win32') + @mock.patch('scrapy.utils.display._enable_windows_terminal_processing') + @mock.patch('platform.version') + @mock.patch("sys.stdout.isatty") + def test_pformat_windows(self, isatty, version, terminal_processing): + isatty.return_value = True + version.return_value = '10.0.14393' + terminal_processing.return_value = True + self.assertEqual(pformat(self.object), self.colorized_string) + + @mock.patch('sys.platform', 'linux') + @mock.patch("sys.stdout.isatty") + def test_pformat_no_pygments(self, isatty): + isatty.return_value = True + + import builtins + real_import = builtins.__import__ + + def mock_import(name, globals, locals, fromlist, level): + if 'pygments' in name: + raise ImportError + return real_import(name, globals, locals, fromlist, level) + + builtins.__import__ = mock_import + self.assertEqual(pformat(self.object), self.plain_string) + builtins.__import__ = real_import + + def test_pprint(self): + with mock.patch('sys.stdout', new=StringIO()) as mock_out: + pprint(self.object) + self.assertEqual(mock_out.getvalue(), "{'a': 1}\n") From 8fae3d5bb768ebb3f9674cc38172226f404fb297 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Wed, 22 Jul 2020 16:08:35 -0300 Subject: [PATCH 56/56] Remove monkeypatches module from mypy section in setup.cfg --- setup.cfg | 3 --- 1 file changed, 3 deletions(-) diff --git a/setup.cfg b/setup.cfg index 46a3d13fc..f8e7c0c91 100644 --- a/setup.cfg +++ b/setup.cfg @@ -13,9 +13,6 @@ follow_imports = skip [mypy-scrapy] ignore_errors = True -[mypy-scrapy._monkeypatches] -ignore_errors = True - [mypy-scrapy.commands] ignore_errors = True