#44 Incoherent behavior with the cache

_locales/en/messages.json

@@ -279,6 +279,11 @@
     "description": ""
   },
 
+  "already-downloaded": {
+    "message": "Already found / downloaded",
+    "description": ""
+  },
+
   "unexpected-small-size": {
     "message": "Unexpected small size",
     "description": ""

_locales/fr/messages.json

@@ -279,6 +279,11 @@
     "description": ""
   },
 
+  "already-downloaded": {
+    "message": "Déjà trouvé / téléchargé",
+    "description": ""
+  },
+
   "unexpected-small-size": {
     "message": "Taille de fichier inattendue",
     "description": ""

src/html/download-list.html

@@ -49,6 +49,12 @@
 	border-radius: 8px;
 }
 
+.already-downloaded {
+	background-color: white;
+	color: black;
+	border-radius: 8px;
+}
+
 .mixed, .downloading {
 	background-color: orange;
 	color: orange;

src/scripts/background/dl-manager.js

@@ -143,15 +143,24 @@ function newDlManager(queue) {
    */
   function startDownload(linkObject, processor) {
 
+    // Only waiting links have to be considered
+    // (VS. already downloaded)
+    if (linkObject.status !== DlStatus.WAITING) {
+      updateProcessorInDownloadView(processor);
+      queue.processNextItem();
+      return;
+    }
+
     // Can we start the download right now?
     // Or should we make it wait?
     if (dlManager.maxDownloadLimit > 0 &&
-        dlManager.ongoingDownloadsCpt >= dlManager.maxDownloadLimit) {
+          dlManager.ongoingDownloadsCpt >= dlManager.maxDownloadLimit) {
 
       dlManager.waitingDownloads.push({
         link: linkObject,
         processor: processor
       });
+
       return;
     }
 

src/scripts/background/library.processors.js

@@ -188,6 +188,7 @@ function onFoundLinks(processor, links, queue, startDownloadFn, updateProcessorI
 
   // If we have links, let things go on
   if (!! links && links.length > 0) {
+    var avoidDuplicatesInThisProcessor = [];
     links.forEach( function(link, index) {
 
       // A found link might be relative.
@@ -200,20 +201,28 @@ function onFoundLinks(processor, links, queue, startDownloadFn, updateProcessorI
         fixedLink = fixedLink.replace(interceptorRegex, interceptor.by);
       });
 
-      // Was the link already downloaded?
-      if (alreadyVisitedUrls.enabled && alreadyVisitedUrls.list.indexOf(fixedLink) !== -1) {
+      // Did we already find it in this processor?
+      // Stop here.
+      if (avoidDuplicatesInThisProcessor.indexOf(fixedLink) !== -1) {
         return;
       }
 
-      // Add the download link
+      avoidDuplicatesInThisProcessor.push(fixedLink);
+
+      // Was the link already downloaded or found in another processor?
+      // Keep it and mark it, so that the user understands.
+      var already = false;
       if (alreadyVisitedUrls.enabled) {
-        alreadyVisitedUrls.list.push(fixedLink);
+        already = alreadyVisitedUrls.list.indexOf(fixedLink) !== -1;
+        if (! already) {
+          alreadyVisitedUrls.list.push(fixedLink);
+        }
       }
 
       processor.downloadLinks.push({
         id: processor.id + '-' + index,
         link: fixedLink,
-        status: DlStatus.WAITING
+        status: already ? DlStatus.ALREADY_DOWNLOADED : DlStatus.WAITING
       });
     });
 

src/scripts/content/library.list.js

@@ -15,6 +15,7 @@ function findClassNameFromStatus(dlLink) {
   case DlStatus.INVALID_MIME_TYPE: result = 'invalid-mime-type'; break;
   case DlStatus.UNEXPECTED_SMALL_SIZE: result = 'unexpected-small-size'; break;
   case DlStatus.DOWNLOADING: result = 'downloading'; break;
+  case DlStatus.ALREADY_DOWNLOADED: result = 'already-downloaded'; break;
   }
 
   return result;
@@ -32,6 +33,7 @@ function findClassNameFromProcessor(processor) {
     switch(dlLink.status) {
     case DlStatus.FAILURE:  ko ++; break;
     case DlStatus.SUCCESS: result = ok ++; break;
+    case DlStatus.ALREADY_DOWNLOADED: result = ok ++; break;
     case DlStatus.WAITING: result = waiting ++; break;
     default: /* We do not care about invalid, etc). */ break;
     }

src/scripts/mixed/library.commons.js

@@ -54,7 +54,8 @@ const DlStatus = {
   FAILURE: 3,
   DOWNLOADING: 4,
   INVALID_MIME_TYPE: 5,
-  UNEXPECTED_SMALL_SIZE: 6
+  UNEXPECTED_SMALL_SIZE: 6,
+  ALREADY_DOWNLOADED: 7
 };
 
 const supportedDictionarySpecs = ['1.0'];

tests/scripts/background/test.library.processors.js

@@ -1205,6 +1205,53 @@ describe('background => library.processors', function() {
       }
     };
 
+    var extractor = {
+      invoked: false,
+      xpath: function(doc, xpathExpr) {
+        this.invoked = true;
+        // Put some duplicate here => unified in the resulting array
+        return ['test1', 'test2'];
+      }
+    };
+
+    // Add an already visited URL
+    var alreadyVisitedUrls = newAlreadyVisitedUrls();
+    alreadyVisitedUrls.list.push('http://localhost:9876/base/tests/resources/test2');
+
+    // Execute the test
+    expect(p.downloadLinks.length).to.eql(0);
+    handleProcessor(p, extractor, queue, idleFn, idleFn, alreadyVisitedUrls);
+
+    // In this case, processing is asynchronous.
+    return sleep(1000).then(function() {
+      expect(extractor.invoked).to.be(true);
+      expect(p.status).to.eql(ProcessorStatus.GOT_LINKS);
+      expect(p.downloadLinks.length).to.eql(2);
+
+      expect(p.downloadLinks[0].link).to.eql('http://localhost:9876/base/tests/resources/test1');
+      expect(p.downloadLinks[0].status).to.eql(DlStatus.WAITING);
+      expect(p.downloadLinks[1].link).to.eql('http://localhost:9876/base/tests/resources/test2');
+      expect(p.downloadLinks[1].status).to.eql(DlStatus.ALREADY_DOWNLOADED);
+      expect(queue.modified).to.eql(false);
+    });
+  });
+
+
+  // No "done "callback for this test.
+  // Instead, we return a promise. If it fails, it will fail the test.
+  it('should handle processors correctly (XPATH with duplicate)', function() {
+
+    var p = newProcessor('test', 'xpath: //test', 'origin url');
+    p.matchingUrl = 'http://localhost:9876/base/tests/resources/empty.html';
+
+    var idleFn = function() {};
+    var queue = {
+      modified: false,
+      processNextItem: function() {
+        this.modified = true;
+      }
+    };
+
     var extractor = {
       invoked: false,
       xpath: function(doc, xpathExpr) {
@@ -1251,12 +1298,13 @@ describe('background => library.processors', function() {
       xpath: function(doc, xpathExpr) {
         this.invoked = true;
         // Put some duplicate here => unified in the resulting array
-        return ['test1', 'test1'];
+        return ['test1', 'test2'];
       }
     };
 
     var cache = newAlreadyVisitedUrls();
     cache.enabled = false;
+    cache.list.push('http://localhost:9876/base/tests/resources/test2');
 
     // Execute the test
     expect(p.downloadLinks.length).to.eql(0);
@@ -1270,7 +1318,7 @@ describe('background => library.processors', function() {
 
       expect(p.downloadLinks[0].link).to.eql('http://localhost:9876/base/tests/resources/test1');
       expect(p.downloadLinks[0].status).to.eql(DlStatus.WAITING);
-      expect(p.downloadLinks[1].link).to.eql('http://localhost:9876/base/tests/resources/test1');
+      expect(p.downloadLinks[1].link).to.eql('http://localhost:9876/base/tests/resources/test2');
       expect(p.downloadLinks[1].status).to.eql(DlStatus.WAITING);
       expect(queue.modified).to.eql(false);
     });

tests/scripts/content/test.library.list.js

@@ -4,12 +4,13 @@ describe('donwload view => list.functions', function() {
 
   it('should find class names from a download link', function(done) {
 
-    expect(findClassNameFromStatus({status: 1})).to.eql('waiting');
-    expect(findClassNameFromStatus({status: 2})).to.eql('success');
-    expect(findClassNameFromStatus({status: 3})).to.eql('failure');
-    expect(findClassNameFromStatus({status: 4})).to.eql('downloading');
-    expect(findClassNameFromStatus({status: 5})).to.eql('invalid-mime-type');
-    expect(findClassNameFromStatus({status: 6})).to.eql('unexpected-small-size');
+    expect(findClassNameFromStatus({status: DlStatus.WAITING})).to.eql('waiting');
+    expect(findClassNameFromStatus({status: DlStatus.SUCCESS})).to.eql('success');
+    expect(findClassNameFromStatus({status: DlStatus.FAILURE})).to.eql('failure');
+    expect(findClassNameFromStatus({status: DlStatus.DOWNLOADING})).to.eql('downloading');
+    expect(findClassNameFromStatus({status: DlStatus.INVALID_MIME_TYPE})).to.eql('invalid-mime-type');
+    expect(findClassNameFromStatus({status: DlStatus.UNEXPECTED_SMALL_SIZE})).to.eql('unexpected-small-size');
+    expect(findClassNameFromStatus({status: DlStatus.ALREADY_DOWNLOADED})).to.eql('already-downloaded');
     expect(findClassNameFromStatus({status: 5748})).to.eql('');
     done();
   });
@@ -40,7 +41,7 @@ describe('donwload view => list.functions', function() {
   it('should find class names from a processor (all failed)', function(done) {
     var processor = {
       status: 0,
-      downloadLinks: [{status: 3}, {status: 3}]
+      downloadLinks: [{status: DlStatus.FAILURE}, {status: DlStatus.FAILURE}]
     };
 
     expect(findClassNameFromProcessor(processor)).to.eql('failure');
@@ -51,7 +52,7 @@ describe('donwload view => list.functions', function() {
   it('should find class names from a processor (all succeeded)', function(done) {
     var processor = {
       status: 0,
-      downloadLinks: [{status: 2}, {status: 2}]
+      downloadLinks: [{status: DlStatus.SUCCESS}, {status: DlStatus.SUCCESS}]
     };
 
     expect(findClassNameFromProcessor(processor)).to.eql('success');
@@ -62,7 +63,7 @@ describe('donwload view => list.functions', function() {
   it('should find class names from a processor (some are still waiting)', function(done) {
     var processor = {
       status: 0,
-      downloadLinks: [{status: 2}, {status: 1}, {status: 3}]
+      downloadLinks: [{status: DlStatus.SUCCESS}, {status: 1}, {status: DlStatus.FAILURE}]
     };
 
     expect(findClassNameFromProcessor(processor)).to.eql('waiting');
@@ -73,7 +74,7 @@ describe('donwload view => list.functions', function() {
   it('should find class names from a processor (some success and some failures)', function(done) {
     var processor = {
       status: 0,
-      downloadLinks: [{status: 2}, {status: 3}, {status: 2}]
+      downloadLinks: [{status: DlStatus.SUCCESS}, {status: DlStatus.FAILURE}, {status: DlStatus.SUCCESS}]
     };
 
     expect(findClassNameFromProcessor(processor)).to.eql('mixed');
@@ -84,10 +85,21 @@ describe('donwload view => list.functions', function() {
   it('should find class names from a processor (some success and some invalid MIME type)', function(done) {
     var processor = {
       status: 0,
-      downloadLinks: [{status: 2}, {status: 5}, {status: 2}]
+      downloadLinks: [{status: DlStatus.SUCCESS}, {status: DlStatus.INVALID_MIME_TYPE}, {status: DlStatus.SUCCESS}]
     };
 
     expect(findClassNameFromProcessor(processor)).to.eql('mixed');
     done();
   });
+
+
+  it('should find class names from a processor (some success and some already downloaded)', function(done) {
+    var processor = {
+      status: 0,
+      downloadLinks: [{status: DlStatus.SUCCESS}, {status: DlStatus.ALREADY_DOWNLOADED}, {status: DlStatus.SUCCESS}]
+    };
+
+    expect(findClassNameFromProcessor(processor)).to.eql('success');
+    done();
+  });
 });