Refuse an album, tag or place that has no address of its own

CHANGELOG.md

@@ -6,6 +6,7 @@ All notable changes to exposer, newest first. The format follows [Keep a Changel
 
 ### Fixed
 
+- An album, tag or place whose name has no letter or digit a URL can carry, such as `Album|東京`, now fails the build naming it. Its page used to be written over the albums or tags index. Two albums or tags that reduce to the same address, such as `Allgäu` and `Allgau`, now fail the build too, where they were silently merged into one page. A name that only appears on unpublished photos is not affected.
 - An album file whose `cover:` names a photograph below the publication gate now fails the build, naming the file and the rating. The album used to point at an image the site does not contain.
 - Correcting a photograph's creator or copyright in its sidecar now reaches the published images. For a JPEG, the old line stayed stamped into every image until the cache was cleared by hand. Images of photographs that carry a creator or copyright are rendered once more on the next build; their names do not change unless their bytes do.
 - A photograph exiftool could not read, such as one still being copied in or without read permission, is now named with the reason, and is read again on the next build. Its failure used to be remembered until the file's size or modification time changed, so fixing the permissions did not help, and the photograph was left out of the index without a word.

cmd/exposer/index.go

@@ -583,6 +583,42 @@ func validateContract(photo Photo) []string {
 	return problems
 }
 
+// checkSlug fails the build on a name that cannot be an address: one with no
+// ASCII letter or digit in it, such as "東京", whose slug is empty, so that its
+// page would be written over the section's own index; or one whose slug an
+// earlier name already holds ("Allgäu" and "Allgau"), so that one page would
+// overwrite the other and their photographs would merge. owners remembers
+// which name holds which slug; nil checks emptiness alone.
+func checkSlug(kind, name, slug string, owners map[string]string) {
+	if slug == "" {
+		fail("%s %q has no letters or digits a URL can carry, so it has no address of its own; "+
+			"name it with at least one Latin letter or digit", kind, name)
+	}
+	if owners == nil {
+		return
+	}
+	if previous, taken := owners[slug]; taken && previous != name {
+		fail("%ss %q and %q both have the slug %q, so one page would overwrite the other; rename one of them",
+			kind, previous, name, slug)
+	}
+	owners[slug] = name
+}
+
+// checkTagSlugs applies checkSlug to the tags that get a page: those on
+// published photographs. Two tags with one slug would share a listing, and a tag
+// with none would be written over the tags index.
+func checkTagSlugs(photos []Photo) {
+	owners := map[string]string{}
+	for _, photo := range photos {
+		if !photo.Published {
+			continue
+		}
+		for _, tag := range photo.Tags {
+			checkSlug("tag", strings.Join(tag.Path, "|"), tag.Slug, owners)
+		}
+	}
+}
+
 // libraryData is where the library keeps the files that describe the site
 // rather than a photograph: albums (R-3), locations (R-10) and pages (R-13).
 // One folder keeps them out of the way of the photographs, which may sit in
@@ -711,6 +747,7 @@ func loadAlbums(library string, photos []Photo) []Album {
 	sort.Strings(names)
 
 	albums := make([]Album, 0, len(names))
+	slugOwner := map[string]string{}
 	for _, name := range names {
 		meta, hasMeta := declared[name]
 		members := byTag[name]
@@ -735,6 +772,9 @@ func loadAlbums(library string, photos []Photo) []Album {
 		// are slugged allgau -- and the album then had a count, an entry in the
 		// index and no page at all, with the build still passing.
 		slug := slugify(name)
+		if len(published) > 0 {
+			checkSlug("album", name, slug, slugOwner)
+		}
 		if hasMeta {
 			if base := filepath.Base(declaredFile[name]); base != slug+".yaml" {
 				fail("%s: an album file is named for the slug of its tag, so %q belongs in %s.yaml",
@@ -892,6 +932,10 @@ func loadLocations(library string, photos []Photo) []Location {
 			}
 		}
 
+		if count > 0 {
+			checkSlug("location", strings.Join(node, "|"), slug, nil)
+		}
+
 		title := node[len(node)-1]
 		if meta.Title != "" {
 			title = meta.Title
@@ -1109,6 +1153,7 @@ func runIndex(args []string) {
 		byID[photo.ID] = photo.Source
 	}
 
+	checkTagSlugs(photos)
 	albums := loadAlbums(*library, photos)
 	locations := loadLocations(*library, photos)
 

cmd/exposer/index_test.go

@@ -254,6 +254,115 @@ func TestParseDateTimeRefusesWhatIsNotADate(t *testing.T) {
 	}
 }
 
+func publishedWithAlbums(id string, albums ...string) Photo {
+	return Photo{ID: id, Published: true, albumTags: albums}
+}
+
+func TestAlbumWithNoAddressFailsTheBuild(t *testing.T) {
+	// "東京" has no ASCII letter, so its slug is empty and its page would be
+	// written to photos/albums/_index.md, over the album index.
+	message := catchFail(t, func() {
+		loadAlbums(t.TempDir(), []Photo{publishedWithAlbums("a1", "東京")})
+	})
+	if !strings.Contains(message, "東京") || !strings.Contains(message, "no letters or digits") {
+		t.Errorf("failed with %q, want it to name the album and say it has no address", message)
+	}
+}
+
+func TestAlbumsSharingASlugFailTheBuild(t *testing.T) {
+	// slugify drops what ASCII cannot say, so these are one address.
+	message := catchFail(t, func() {
+		loadAlbums(t.TempDir(), []Photo{
+			publishedWithAlbums("a1", "Allgäu"),
+			publishedWithAlbums("a2", "Allgau"),
+		})
+	})
+	for _, want := range []string{"Allgäu", "Allgau", `"allgau"`} {
+		if !strings.Contains(message, want) {
+			t.Errorf("failed with %q, want it to mention %s", message, want)
+		}
+	}
+}
+
+func TestAlbumsNobodyPublishesNeedNoAddress(t *testing.T) {
+	// An album of unpublished photographs has no page, so its slug is not
+	// contended for: failing here would break libraries that build today.
+	hidden := Photo{ID: "a1", Published: false, albumTags: []string{"東京"}}
+	other := Photo{ID: "a2", Published: false, albumTags: []string{"Tokyo", "tokyo"}}
+	if message := catchFail(t, func() { loadAlbums(t.TempDir(), []Photo{hidden, other}) }); message != "" {
+		t.Errorf("an album with no published photograph failed the build: %s", message)
+	}
+}
+
+func TestDistinctAlbumsPass(t *testing.T) {
+	albums := loadAlbums(t.TempDir(), []Photo{
+		publishedWithAlbums("a1", "Harbour", "Allgäu"),
+		publishedWithAlbums("a2", "Harbour"),
+	})
+	if len(albums) != 2 {
+		t.Fatalf("got %d albums, want 2", len(albums))
+	}
+}
+
+func TestTagsSharingASlugFailTheBuild(t *testing.T) {
+	tagged := func(published bool, paths ...[]string) Photo {
+		photo := Photo{Published: published}
+		for _, path := range paths {
+			photo.Tags = append(photo.Tags, Tag{
+				Slug: slugify(strings.Join(path, "-")), Path: path, Leaf: path[len(path)-1],
+			})
+		}
+		return photo
+	}
+
+	// Places|Harbour Dock and Places|Harbour|Dock are both places-harbour-dock.
+	message := catchFail(t, func() {
+		checkTagSlugs([]Photo{
+			tagged(true, []string{"Places", "Harbour Dock"}),
+			tagged(true, []string{"Places", "Harbour", "Dock"}),
+		})
+	})
+	if !strings.Contains(message, "Places|Harbour Dock") || !strings.Contains(message, "Places|Harbour|Dock") {
+		t.Errorf("failed with %q, want it to name both tags", message)
+	}
+
+	// The same tag on many photographs is one tag.
+	if message := catchFail(t, func() {
+		checkTagSlugs([]Photo{
+			tagged(true, []string{"Places", "Harbour"}),
+			tagged(true, []string{"Places", "Harbour"}),
+		})
+	}); message != "" {
+		t.Errorf("one tag on two photographs failed: %s", message)
+	}
+
+	// Nothing is written for a tag on unpublished photographs only.
+	if message := catchFail(t, func() {
+		checkTagSlugs([]Photo{
+			tagged(true, []string{"Places", "Harbour Dock"}),
+			tagged(false, []string{"Places", "Harbour", "Dock"}),
+		})
+	}); message != "" {
+		t.Errorf("a tag on an unpublished photograph failed the build: %s", message)
+	}
+
+	// A tag with no slug would be written over the tags index.
+	message = catchFail(t, func() {
+		checkTagSlugs([]Photo{tagged(true, []string{"Москва"}, []string{"東京"})})
+	})
+	if !strings.Contains(message, "no letters or digits") {
+		t.Errorf("failed with %q, want a tag with an empty slug refused", message)
+	}
+}
+
+func TestLocationWithNoAddressFailsTheBuild(t *testing.T) {
+	photo := Photo{Published: true, locationPaths: [][]string{{"日本"}}}
+	message := catchFail(t, func() { loadLocations(t.TempDir(), []Photo{photo}) })
+	if !strings.Contains(message, "日本") || !strings.Contains(message, "no letters or digits") {
+		t.Errorf("failed with %q, want it to name the location and say it has no address", message)
+	}
+}
+
 func TestAlbumCoverMustBePublished(t *testing.T) {
 	// A cover is drawn from the photographs' derivatives, which exist only for
 	// published ones; an unpublished cover left the album pointing at an image