flow: Merged <feature> 'fix_anchor_duplication' to <develop> ('develop').

docs/TODO.md

@@ -2,11 +2,6 @@
 
 ## Bugs
 
-* deal with anchor issue (repeatedly crawling index.html#about, index.html#contact, etc.)
-  * should only crawl these pages once
-  * but should be included in the links part
-* be smart about index.html?
-
 ## Cleanup
 
 * helper function to do the whole crawl process

src/docrawler/docrawler.go

@@ -12,6 +12,11 @@ func doCrawl(homeurl string) itemSlice {
 	// set of what we have already crawled
 	crawled := make(itemMap)
 
+	// a set of crawled URLs which have been cleaned
+	// useful so that we don't crawl http://a.com/index.html#about if we've
+	// already crawled http://a.com/index.html, or vice versa
+	crawledStripped := make(itemMap)
+
 	// map (of urlString -> httpItem) of our results
 	results := make(itemMap)
 
@@ -32,6 +37,7 @@ func doCrawl(homeurl string) itemSlice {
 
 	// start the home page crawl
 	crawled[homeitem.url.String()] = homeitem
+	crawledStripped[stripURL(homeitem.url)] = homeitem
 	go crawlItem(homeitem, rchan)
 
 	// wait for results
@@ -59,10 +65,21 @@ func doCrawl(homeurl string) itemSlice {
 					r.children[i] = existing
 					continue
 				}
+				if existing, ok := crawledStripped[stripURL(c.url)]; ok {
+					// we crawled a different version of this same page
+					// i.e. same page, different anchor. we don't need to crawl it again
+					// but we do need to keep it in our results, so copy the existing
+					// struct over for everything except the URLs
+					r.children[i].title = existing.title
+					r.children[i].linkType = existing.linkType
+					r.children[i].children = existing.children
+					continue
+				}
 
 				// haven't crawled this one yet, do so now
 				crawlingCount++
 				crawled[c.url.String()] = c
+				crawledStripped[stripURL(c.url)] = c
 				go crawlItem(c, rchan)
 			}
 
@@ -102,7 +119,12 @@ func crawlItem(item *httpItem, rchan chan<- *httpItem) {
 	}
 
 	// parse links
-	title, links := parseLinks(text)
+	title, links := parseLinks(text) // TODO remove error from parseLinks, not needed
+	if err != nil {
+		item.linkType = tBroken
+		rchan <- item
+		return
+	}
 	item.title = title
 
 	// walk links and add them as children to the current item

src/docrawler/fetch.go

@@ -74,13 +74,11 @@ func fetchPage(item *httpItem) (string, error) {
 	// GET the url
 	resp, err := http.Get(item.url.String())
 	if err != nil {
-		// NOTE http.Head was already successful, this is highly likely to succeed
 		return "", err
 	}
 
 	// check response code
 	if resp.StatusCode != http.StatusOK {
-		// NOTE we already got StatusOK for http.Head, this is highly likely to succeed
 		return "", errFetchError
 	}
 

src/docrawler/url.go

@@ -51,14 +51,26 @@ func resolveURL(referringURL string, currentURL string) (*url.URL, error) {
 	return uResolved, checkURL(uResolved)
 }
 
-// stripAnchorFromURL returns a version of the URL with out the "Fragment" part,
+// stripURL returns a version of the URL without the "Fragment" part,
 // which is anything after the '#' character, so:
 // http://a.com/blah.html#anchor becomes http://a.com/blah.html
-func stripAnchorFromURL(u *url.URL) string {
+// ... and without files like "index.htm" in the Path (since that's
+// equivalent to "/"
+func stripURL(u *url.URL) string {
 	// copy the url locally
 	ucopy := *u
 
 	// blank out the anchor, return URI string
 	ucopy.Fragment = ""
+
+	// remove index.html if it exists
+	suffixes := []string{"index.html", "index.htm"}
+	for _, s := range suffixes {
+		if strings.HasSuffix(strings.ToLower(ucopy.Path), s) {
+			ucopy.Path = ucopy.Path[0 : len(ucopy.Path)-len(s)]
+		}
+	}
+
+	// return new stripped down URL string
 	return ucopy.String()
 }

src/docrawler/url_test.go

@@ -69,14 +69,36 @@ func TestBadResolve(t* testing.T) {
 }
 */
 
-// TestURLStripAnchor tests the stripAnchorFromURL() function
+// TestURLStripAnchor tests the stripURL() function
 func TestURLStripAnchor(t *testing.T) {
-	u, err := url.Parse("http://swaits:pass@somehost.com:8765/blah/blah.html?x=y#foo")
-	if err != nil {
-		t.Fatal("couldn't parse a URL")
+	{
+		u, err := url.Parse("http://swaits:pass@somehost.com:8765/blah/blah.html?x=y#foo")
+		if err != nil {
+			t.Fatal("couldn't parse a URL")
+		}
+		stripped := stripURL(u)
+		if stripped != "http://swaits:pass@somehost.com:8765/blah/blah.html?x=y" {
+			t.Error("stripping anchor #foo from URL failed")
+		}
 	}
-	stripped := stripAnchorFromURL(u)
-	if stripped != "http://swaits:pass@somehost.com:8765/blah/blah.html?x=y" {
-		t.Error("stripping anchor #foo from URL failed")
+	{
+		u, err := url.Parse("http://swaits:pass@somehost.com:8765/blah/index.html?x=y#foo")
+		if err != nil {
+			t.Fatal("couldn't parse a URL")
+		}
+		stripped := stripURL(u)
+		if stripped != "http://swaits:pass@somehost.com:8765/blah/?x=y" {
+			t.Error("stripping anchor #foo from URL failed")
+		}
+	}
+	{
+		u, err := url.Parse("http://swaits:pass@somehost.com:8765/blah/index.htm?x=y#foo")
+		if err != nil {
+			t.Fatal("couldn't parse a URL")
+		}
+		stripped := stripURL(u)
+		if stripped != "http://swaits:pass@somehost.com:8765/blah/?x=y" {
+			t.Error("stripping anchor #foo from URL failed")
+		}
 	}
 }