flow: Closed <feature> 'make_json_pretty'.

* so now http://a.com/index.html and http://a.com/ are understood equivalents
* as are http://a.com/x.html#blog and http://a.com/x.html (etc.)
* this meant a little more bookkeeping in docrawler, but not horrible
* stripAnchorFromURL because stripURL because it now handles anchor and path suffix stripping
* and url tests were written to support this

docs/Design.md

@@ -2,38 +2,38 @@
 
 ## Minimum Features
 
-This is the assignment (directly) from DO.
+This is the assignment (directly) from **REDACTED**.
 
-* We do ask that you put a heavy emphasis on testing in the assignment.
-* written in a modern language
-* limited to one domain - so when crawling digitalocean.com it would crawl all pages within the digitalocean.com domain, but not follow the links to our Facebook or Instagram accounts or subdomains like cloud.digitalocean.com. 
-* It should be Given a URL
-* it should output a site map
-* it should show which static assets each page depends on, and the links between pages.
-* Choose the most appropriate data structure to store & display this site map.
-* Build this as you would build something for production - it's fine if you don't finish everything, but focus on code quality and write tests.
-* We're interested in how you code and how you test your code.
+✓ We do ask that you put a heavy emphasis on testing in the assignment.
+ written in a modern language
+✓ limited to one domain - so when crawling REDACTED.com it would crawl all pages within the REDACTED.com domain, but not follow the links to our Facebook or Instagram accounts or subdomains like cloud.REDACTED.com. 
+✓ It should be Given a URL
+✓ it should output a site map
+✓ it should show which static assets each page depends on, and the links between pages.
+✓ Choose the most appropriate data structure to store & display this site map.
+✓ Build this as you would build something for production - it's fine if you don't finish everything, but focus on code quality and write tests.
+✓ We're interested in how you code and how you test your code.
 
 ## Desired
 
 These are features I'd like to include.
 
 * efficient, optimal
-* handles base URL properly (for root & relative links)
-* documented
-* uses standard Go practices
-* uses Go stdlib (only!)
-* thoroughly tested
+✓ handles base URL properly (for root & relative links)
+✓ documented
+✓ uses standard Go practices
+✓ uses Go stdlib (only!)
+✓ thoroughly tested
 * configurable on command line (defaults are for company specs)
 * variable output formats: json, dot
-* adheres to URL RFC (as far as case sensitivity, acceptable character sets, etc.)
+✓ adheres to URL RFC (as far as case sensitivity, acceptable character sets, etc.)
 * go gettable
-* go fmt'ed, vet'ed, lint'ed
+✓ go fmt'ed, vet'ed, lint'ed
 * polite - robots.txt support
-* robust - detect infinite loops
+✓ robust - detect infinite loops
 * throttling
 * use Go context pattern
-* supports http & https
+✓ supports http & https
 
 ## Architecture
 
@@ -41,8 +41,8 @@ These are features I'd like to include.
 * pipeline chain should be such that when there's no more work to do the job dispatcher closes the next channel in the pipeline, thereby closing the entire chain of channels (and exiting all goroutines)
 * pipeline may allows us to scale concurrency at the bottlenecks
 * could use Interfaces for things like Parser, Getter, but I think best avoided until needed
-* launch goroutine per fetch
-* when there's no Jobs left, and nothing being fetched or parsed, convert the data struct into something useful and output it
+✓ launch goroutine per fetch
+✓ when there's no Jobs left, and nothing being fetched or parsed, convert the data struct into something useful and output it
 
 ## Wishlist
 

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)
 			}
 

src/docrawler/docrawler_test.go

@@ -229,7 +229,38 @@ func TestJsonOutput(t *testing.T) {
 	// this is a cheater test because the output is from a run of the code being
 	// test itself. but, it's been examined and I think it's right. and, there's not
 	// many other ways to test this without some significant pain
-	cheaterTest := `[{"URL":"http://localhost:8765/","Title":"Home","Links":["http://localhost:8765/about.html"],"Assets":["http://localhost:8765/assets/image.png","http://localhost:8765/scripts/blah.js"],"Broken":["http://localhost:8765/zzzbroken.html"],"Remote":null},{"URL":"http://localhost:8765/about.html","Title":"About Test","Links":["http://localhost:8765/"],"Assets":["http://localhost:8765/assets/image.png","http://localhost:8765/scripts/blah.js"],"Broken":null,"Remote":["http://doesntexist23492387492837492374982734.com/"]}]`
+	cheaterTest := `[
+  {
+    "URL": "http://localhost:8765/",
+    "Title": "Home",
+    "Links": [
+      "http://localhost:8765/about.html"
+    ],
+    "Assets": [
+      "http://localhost:8765/assets/image.png",
+      "http://localhost:8765/scripts/blah.js"
+    ],
+    "Broken": [
+      "http://localhost:8765/zzzbroken.html"
+    ],
+    "Remote": null
+  },
+  {
+    "URL": "http://localhost:8765/about.html",
+    "Title": "About Test",
+    "Links": [
+      "http://localhost:8765/"
+    ],
+    "Assets": [
+      "http://localhost:8765/assets/image.png",
+      "http://localhost:8765/scripts/blah.js"
+    ],
+    "Broken": null,
+    "Remote": [
+      "http://doesntexist23492387492837492374982734.com/"
+    ]
+  }
+]`
 	j, err := locationsToJSON(l)
 	if err != nil {
 		t.Error("locationsToJSON failed")

src/docrawler/output.go

@@ -79,7 +79,7 @@ func sitemapToLocations(pages itemSlice) []*Location {
 
 // locationsToJSON takes a *Location slice and marshals it into a JSON string
 func locationsToJSON(locations []*Location) (string, error) {
-	b, err := json.Marshal(locations)
+	b, err := json.MarshalIndent(locations, "", "  ")
 	if err != nil {
 		return "", err
 	}

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")
+		}
 	}
 }