flow: Closed <feature> 'cleanup_and_refactoring'.

* moved crawlItem into fetch.go
* renamed fetchPage fetchItem
* doCrawl now takes number of workers as a parameter
* command line flag parsing added

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
 

src/docrawler/docrawler.go

@@ -1,15 +1,16 @@
 package main
 
 import (
+	"flag"
 	"fmt"
 	"log"
 	"os"
 	"time"
 )
 
-// docrawl begins crawling the site at "homeurl"
-func doCrawl(homeurl string) itemSlice {
-	// set of what we have already crawled
+// doCrawl begins crawling the site at "homeurl"
+func doCrawl(homeurl string, nWorkers int) itemSlice {
+	// set of what we have already crawled, our results
 	crawled := make(itemMap)
 
 	// a set of crawled URLs which have been cleaned
@@ -17,11 +18,14 @@ func doCrawl(homeurl string) itemSlice {
 	// already crawled http://a.com/index.html, or vice versa
 	crawledStripped := make(itemMap)
 
-	// map (of urlString -> httpItem) of our results
-	results := make(itemMap)
+	// channels to send work to and receive our results on
+	rxchan := make(chan *httpItem)
+	txchan := make(chan *httpItem)
 
-	// a channel to receive our results on
-	rchan := make(chan *httpItem)
+	// spin up our crawler workers
+	for i := 0; i < nWorkers; i++ {
+		go crawlWorker(txchan, rxchan)
+	}
 
 	// start a ticker which we'll use to output status and check for completion
 	ticker := time.Tick(250 * time.Millisecond)
@@ -29,7 +33,7 @@ func doCrawl(homeurl string) itemSlice {
 	// create first page's httpItem
 	homeitem, err := newHTTPItem(nil, homeurl)
 	if err != nil {
-		log.Fatal("unable to create httpItem for homeurl")
+		return nil
 	}
 
 	// set our number of outstanding pages (initially 1 to account for first page)
@@ -38,14 +42,14 @@ func doCrawl(homeurl string) itemSlice {
 	// start the home page crawl
 	crawled[homeitem.url.String()] = homeitem
 	crawledStripped[stripURL(homeitem.url)] = homeitem
-	go crawlItem(homeitem, rchan)
+	txchan <- homeitem
 
 	// wait for results
 	for {
 		select {
-		case r := <-rchan: // new results?
+		case r := <-rxchan: // new results?
 			// add result to our results map
-			results[r.url.String()] = r
+			crawled[r.url.String()] = r
 
 			// decrease the outstanding page count by 1
 			crawlingCount--
@@ -54,7 +58,7 @@ func doCrawl(homeurl string) itemSlice {
 			for i, c := range r.children {
 				// see if we already have a result for this page
 				// if so, point to that result
-				if existing, ok := results[c.url.String()]; ok {
+				if existing, ok := crawled[c.url.String()]; ok {
 					r.children[i] = existing
 					continue
 				}
@@ -80,18 +84,24 @@ func doCrawl(homeurl string) itemSlice {
 				crawlingCount++
 				crawled[c.url.String()] = c
 				crawledStripped[stripURL(c.url)] = c
-				go crawlItem(c, rchan)
+				go func(newItem *httpItem) {
+					txchan <- newItem
+				}(c)
 			}
 
 		case <-ticker: // our regular ticker. for status output and checking for completion.
 			// output status to console
-			log.Printf("Crawled %v links, have %v left.\n", len(results), crawlingCount)
+			log.Printf("Crawled %v links, have %v left.\n", len(crawled), crawlingCount)
 
 			// see if we're finished
 			if crawlingCount == 0 {
+				// close our channels, signalling any workers to exit
+				close(txchan)
+				close(rxchan)
+
 				// finished! convert results map to a slice and return it
 				rslice := itemSlice{}
-				for _, v := range results {
+				for _, v := range crawled {
 					rslice = append(rslice, v)
 				}
 				return rslice
@@ -100,50 +110,39 @@ func doCrawl(homeurl string) itemSlice {
 	}
 }
 
-// crawlItem crawls a single httpItem, fetching the header, hte page, parsing it,
-// and filling out its structure as much as possible
-func crawlItem(item *httpItem, rchan chan<- *httpItem) {
-	// make sure this item is the same domain (i.e. URL "host part") as its referrer
-	if item.refurl != nil && item.url.Host != item.refurl.Host {
-		// skip URLs associated with other Hosts
-		item.linkType = tRemote
-		rchan <- item
-		return
+// crawlWorker is a goroutine'ized wrapper around crawlItem that listens
+// for new jobs and sends them off to crawlItem, returning the results in rxchan
+func crawlWorker(txchan <-chan *httpItem, rxchan chan<- *httpItem) {
+	// read off the incoming item channel forever
+	for newJob := range txchan {
+		// perform the crawl
+		crawlItem(newJob)
+		// return the result
+		rxchan <- newJob
 	}
-
-	// fetch page
-	text, err := fetchPage(item)
-	if err != nil {
-		rchan <- item
-		return
-	}
-
-	// parse links
-	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
-	for _, l := range links {
-		newItem, err := newHTTPItem(item, l)
-		if err != nil {
-			continue // TODO bad item
-		}
-		item.children = append(item.children, newItem)
-	}
-
-	// send back our item struct now that it's all filled out
-	rchan <- item
 }
 
+// main is our program's entry point
 func main() {
+	// parse our flags
+	nWorkers := *(flag.Uint("num", 100, "number of workers"))
+	// see if we've got no arguments
+	flag.Parse()
+	if flag.NArg() < 1 {
+		fmt.Printf("\nerror: Please specify at least one URL to crawl.\n")
+		fmt.Printf("\nD.O. Crawler 1.0  Copyright (c) 2015 Stephen Waits <steve@waits.net>  2015-02-17\n\n")
+		fmt.Printf("usage: %v [-num=100] <URLs...>\n\n", os.Args[0])
+		fmt.Printf("  -num=100: number of workers\n")
+		fmt.Printf("  URLs:     URLs to crawl\n\n")
+		os.Exit(1)
+	}
+
 	// crawl each URL on the command line
-	for _, u := range os.Args[1:] {
-		pages := doCrawl(u)
+	for _, u := range flag.Args() {
+		pages := doCrawl(u, int(nWorkers))
+		if pages == nil {
+			log.Fatalf("unable to crawl %q, may be an invalid URL\n", u)
+		}
 		l := sitemapToLocations(pages)
 		j, _ := locationsToJSON(l)
 		fmt.Println(j)

src/docrawler/docrawler_test.go

@@ -62,7 +62,7 @@ func TestSimpleFetch(t *testing.T) {
 	if err != nil {
 		t.Error("problem creating New httpItem struct")
 	}
-	body, err := fetchPage(page)
+	body, err := fetchItem(page)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -76,7 +76,7 @@ func TestSimpleFetch(t *testing.T) {
 
 // TestSimpleMap figures out the site map for the site in baseURL
 func TestSimpleMap(t *testing.T) {
-	pages := doCrawl(baseURL)
+	pages := doCrawl(baseURL, 10)
 
 	// because our crawl is non-deterministic, we have to do a complete
 	// cycle through every page, counting stuff, finding specific pages
@@ -185,7 +185,7 @@ func TestHeaderFetching(t *testing.T) {
 
 // TestJsonOutput gets a sitemap and then converts it to json
 func TestJsonOutput(t *testing.T) {
-	pages := doCrawl(baseURL)
+	pages := doCrawl(baseURL, 10)
 	l := sitemapToLocations(pages)
 	if len(l) != 2 {
 		t.Error("sitemapToLocations has the wrong number of locations")
@@ -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/fetch.go

@@ -54,7 +54,7 @@ func fetchFiletype(item *httpItem) error {
 }
 
 // fetchPage takes a httpItem, GETs it, and returns the body as a string
-func fetchPage(item *httpItem) (string, error) {
+func fetchItem(item *httpItem) (string, error) {
 	// figure out the file type
 	err := fetchFiletype(item)
 	if err != nil {
@@ -92,3 +92,33 @@ func fetchPage(item *httpItem) (string, error) {
 	resp.Body.Close()
 	return string(body), nil
 }
+
+// crawlItem crawls a single httpItem, fetching the header, hte page, parsing it,
+// and filling out its structure as much as possible
+func crawlItem(item *httpItem) {
+	// make sure this item is the same domain (i.e. URL "host part") as its referrer
+	if item.refurl != nil && item.url.Host != item.refurl.Host {
+		// skip URLs associated with other Hosts
+		item.linkType = tRemote
+		return
+	}
+
+	// fetch page
+	text, err := fetchItem(item)
+	if err != nil {
+		return
+	}
+
+	// parse links
+	title, links := parseLinks(text)
+	item.title = title
+
+	// walk links and add them as children to the current item
+	for _, l := range links {
+		newItem, err := newHTTPItem(item, l)
+		if err != nil {
+			continue // TODO bad item
+		}
+		item.children = append(item.children, newItem)
+	}
+}

src/docrawler/fetch_test.go

@@ -21,7 +21,7 @@ func TestFailedFetching(t *testing.T) {
 	if err != nil {
 		t.Error("problem creating New Page struct")
 	}
-	if _, err := fetchPage(page); err == nil {
+	if _, err := fetchItem(page); err == nil {
 		t.Error("tired fetching bogus page but didn't get nil back from fetchPage")
 	}
 }

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
 	}