Repository: scrapy/scrapy
Pull request: fix: return unique_list only when link_extractor.unique is True
In the `lxmlhtml.py` script, the function `extract_links(self, response)` always returns `unique_list(all_links)`, even when the attribute `unique` of the `link_extractor` is equal to `False`. 

The behavior is coherent with the `unique` attribute with this little change in the return line.

Fixes https://github.com/scrapy/scrapy/issues/3798, closes https://github.com/scrapy/scrapy/pull/3799, closes https://github.com/scrapy/scrapy/pull/4695 

Problem statement:
LinkExtractor with Unique = False doesn't extract fully identical Links
Case: 
unexpected behavior identified for LinkExctractor with Unique = False if the page contains fully identical links (the same URL and text).
The current result returns one unique link instead of two or more identical ones.
For example:

1) Local http file 

```<your_path>Test_file.html
<html>
<body>
<a href='sample3.html'>sample 3 repetition</a>
<a href='sample3.html'>sample 3 repetition</a>
</body>
</html>```

2) Scrapy test code:
ignore the error about <GET file:///robots.txt>  as we use local file

`import scrapy
from scrapy.linkextractors import LinkExtractor

class QuotesSpider(scrapy.Spider):
    name = “test”
    start_urls = [
        'file:///<insert_your_local_path>/Test_file.html',
    ]

def __init__(self, *args, **kwargs):
    super(QuotesSpider, self).__init__(*args, **kwargs)
    self.le = LinkExtractor(unique=False)

def parse(self, response):
    links = self.le.extract_links(response)
    yield {'extract_link': links}`

3) run the code: scrapy crawl test, and get the result with one link:

{'extract_link': [Link(url='file:///<your_local_path>/sample3.html', text='sample 3 repetition', fragment='', nofollow=False)]}

Instead of the result with two links from HTML:

{'extract_link': [Link(url='file:///<your_local_path>/sample3.html', text='sample 3 repetition', fragment='', nofollow=False), 
Link(url='<your_local_path>/sample3.html', text='sample 3 repetition', fragment='', nofollow=False)]}

Hints from issue discussion:
Pre-solution: 
the issue is driven by the public method exctract_link of LxmlLinkExtractor(the same as LinkExctractor) which returns unique_list(all_links), see line 130 in module lxmlhtml.py, instead of all_links (similar to solution in sgml.py).

However, here we get another issue:
It looks like the original idea of this unique_list(all_links) was to avoid duplication of links if there is a repetition of tags in restrict_css and restrict_xpaths stated by user In LinkExctractor instance.  
For example: self.le =   ##LinkExtractor(unique=False, restrict_css=('a', 'a'), restrict_xpaths=('a', ‘a’)).

The duplication of  restrict_css and restrict_xpaths  is prosed to be corrected by applying function unique_list() to joined/combined list of css and x_paths in __init__.py module of class FilteringLinkExtractor(object).
Code for correction:

a) Initial code for joined css_x_paths: 
scrapy/linkextractors/__init__.py

self.restrict_xpaths = tuple(arg_to_iter(restrict_xpaths))
self.restrict_xpaths += tuple(map(self._csstranslator.css_to_xpath,
                                   arg_to_iter(restrict_css)))

b) Adjusted code for joined css_x_paths: 
scrapy/linkextractors/__init__.py

_joined_xpath_css = arg_to_iter(restrict_xpaths)
_joined_xpath_css += tuple((map(self._csstranslator.css_to_xpath,
                                  arg_to_iter(restrict_css))))
self.restrict_xpaths = tuple(unique_list(_joined_xpath_css))

Commit message: Fix tests for allowing duplicates in extract_links

Patch to review:
diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py
index 1e6ab984a72..8a4175d49ee 100644
--- a/scrapy/linkextractors/lxmlhtml.py
+++ b/scrapy/linkextractors/lxmlhtml.py
@@ -195,7 +195,8 @@ def extract_links(self, response):
         Only links that match the settings passed to the ``__init__`` method of
         the link extractor are returned.
 
-        Duplicate links are omitted.
+        Duplicate links are omitted if the ``unique`` parameter is set to ``True``, 
+        otherwise they are returned.
         """
         base_url = get_base_url(response)
         if self.restrict_xpaths:
@@ -210,4 +211,6 @@ def extract_links(self, response):
         for doc in docs:
             links = self._extract_links(doc, response.url, response.encoding, base_url)
             all_links.extend(self._process_links(links))
-        return unique_list(all_links)
+        if self.link_extractor.unique:
+            return unique_list(all_links)
+        return all_links
diff --git a/tests/sample_data/link_extractor/linkextractor.html b/tests/sample_data/link_extractor/linkextractor.html
index e3a2a4145e7..29075602d5b 100644
--- a/tests/sample_data/link_extractor/linkextractor.html
+++ b/tests/sample_data/link_extractor/linkextractor.html
@@ -13,6 +13,7 @@
       </div>
       <a href='http://example.com/sample3.html' title='sample 3'>sample 3 text</a>
       <a href='sample3.html'>sample 3 repetition</a>
+      <a href='sample3.html'>sample 3 repetition</a>
       <a href='sample3.html#foo'>sample 3 repetition with fragment</a>
       <a href='http://www.google.com/something'></a>
       <a href='http://example.com/innertag.html'><strong>inner</strong> tag</a>
diff --git a/tests/test_http_response.py b/tests/test_http_response.py
index 74e170ec090..07aef2ee194 100644
--- a/tests/test_http_response.py
+++ b/tests/test_http_response.py
@@ -541,6 +541,7 @@ def test_follow_selector(self):
             'http://example.com/sample2.html',
             'http://example.com/sample3.html',
             'http://example.com/sample3.html',
+            'http://example.com/sample3.html',
             'http://example.com/sample3.html#foo',
             'http://www.google.com/something',
             'http://example.com/innertag.html'
diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py
index e28dc9bdbc8..6c34a96a03e 100644
--- a/tests/test_linkextractors.py
+++ b/tests/test_linkextractors.py
@@ -53,6 +53,7 @@ def test_extract_filter_allow_with_duplicates(self):
                 Link(url='http://example.com/sample2.html', text='sample 2'),
                 Link(url='http://example.com/sample3.html', text='sample 3 text'),
                 Link(url='http://example.com/sample3.html', text='sample 3 repetition'),
+                Link(url='http://example.com/sample3.html', text='sample 3 repetition'),
                 Link(url='http://example.com/sample3.html#foo', text='sample 3 repetition with fragment')
             ])
 
@@ -64,6 +65,7 @@ def test_extract_filter_allow_with_duplicates_canonicalize(self):
                 Link(url='http://example.com/sample2.html', text='sample 2'),
                 Link(url='http://example.com/sample3.html', text='sample 3 text'),
                 Link(url='http://example.com/sample3.html', text='sample 3 repetition'),
+                Link(url='http://example.com/sample3.html', text='sample 3 repetition'),
                 Link(url='http://example.com/sample3.html', text='sample 3 repetition with fragment')
             ])
 


Code review comments:
diff_hunk: @@ -195,7 +195,8 @@ def extract_links(self, response):
         Only links that match the settings passed to the ``__init__`` method of
         the link extractor are returned.
 
-        Duplicate links are omitted.
+        Duplicate links are omitted if the ``unique`` parameter is set to ``True``, 
path: scrapy/linkextractors/lxmlhtml.py
text: @user1:
```suggestion
        Duplicate links are omitted if the ``unique`` attribute is set to ``True``, 
```