Repository: Flutter-Bounty-Hunters/static_shock (dart)

Issue:
[Feature] - Find and report any changed public URLs without a redirect
Any pre-existing public URL might be bookmarked by someone, including the existence of a social media post, which lives forever. Static Shock should support a robust URL forwarding mechanism.

Static Shock already supports the ability to forward a URL, but there are currently no other tools to help identify URLs that need to be forwarded. For example, a user might change a URL from "/guides/getting-started" to "/guides/quickstart" and forget to also define URL forwarding.

Static Shock should add a local accounting system, which tracks all existing URLs as of the previous build, and then ensure that all previous URLs continue to exist in the next build.

I don't have a strong opinion as to how this is done, but here are a few thoughts:
 * Static Shock can create/manage a yaml file (e.g., `urls_public.yaml`) to track all existing public URLs. This file can also document the date the URL was first found by Static Shock. This file can also document which URLs are deprecated (meaning they forward somewhere else).
 * Static Shock can create/manage a yaml file (e.g., `urls_forwarding.yaml`) to configure all URL forwarding for the website. This behavior needs to be de-duped with the existing URL forwarding mechanism. This file would be used during build to actually configure each forwarded URL in the final build.
 * The Static Shock build can output warnings for any public URLs that have disappeared. The user can choose to fail the build or pass the build when this happens. Perhaps the user would like to pass the build locally, but fail the build in CI for such issues.
 * The CLI can check for any URL issues, e.g., `shock urls check`
 * The CLI can add a redirect, e.g., `shock urls redirect --from=/guides/getting-started --to=/guides/quickstart`
 * The CLI can add stubs for all missing public URLs and then the user can type in each redirect in the yaml file, e.g., `shock urls redirect all`.

Pull request:
Expand LinksPlugin to find missing pages since last build (Resolves #184)

CLI: Adds parsing of "--dev" and "--production" to set build mode, which is easier to type than "--build-mode production".

Patch:
diff --git a/packages/static_shock/lib/src/plugins/links.dart b/packages/static_shock/lib/src/plugins/links.dart
index 382da4b..d9fdb75 100644
--- a/packages/static_shock/lib/src/plugins/links.dart
+++ b/packages/static_shock/lib/src/plugins/links.dart
@@ -13,7 +13,10 @@ import 'package:static_shock/src/static_shock.dart';
 class LinksPlugin implements StaticShockPlugin {
   const LinksPlugin({
     this.shouldRunLinkVerification,
-    this.failBuildOnBrokenLinks = false,
+    this.failBuildOnBrokenLinks = true,
+    this.pageManifestUpdatePolicy,
+    this.reportMissingPagesAtErrorLevel,
+    this.includeDraftPagesInPageManifest = false,
   });
 
   /// Whether this plugin should verify all links, or not, or `null` to let
@@ -27,6 +30,27 @@ class LinksPlugin implements StaticShockPlugin {
   /// links are reported to Static Shock as errors, rather than warnings.
   final bool failBuildOnBrokenLinks;
 
+  /// The policy that determines whether the page manifest cache is updated during this build.
+  ///
+  /// When `null`, defers to the current build mode. In "production" the page manifest is updated
+  /// no matter what. In "dev" the page manifest isn't updated at all.
+  ///
+  /// This generation should only be done on publication of the website, because
+  /// the purpose of this manifest is to avoid losing previously published internal
+  /// URLs.
+  final PageManifestUpdatePolicy? pageManifestUpdatePolicy;
+
+  /// The error level to use when reporting pages that are missing from the previous build.
+  ///
+  /// Defaults to "warning" for dev builds, and "error" for production builds.
+  ///
+  /// See the [ErrorLog] to understand the implications of different levels.
+  final StaticShockErrorLevel? reportMissingPagesAtErrorLevel;
+
+  /// Whether draft (not yet published) pages should be included in the list of all
+  /// pages, when checking for missing pages, and when saving a new manifest.
+  final bool includeDraftPagesInPageManifest;
+
   @override
   String get id => "io.staticshock.links";
 
@@ -36,22 +60,42 @@ class LinksPlugin implements StaticShockPlugin {
     StaticShockPipelineContext context,
     StaticShockCache pluginCache,
   ) {
-    if (true == shouldRunLinkVerification ||
-        (shouldRunLinkVerification == null && context.buildMode == StaticShockBuildMode.dev)) {
-      pipeline.finish(
-        BrokenLinkFinderFinisher(
-          reportBrokenLinksAsErrors: failBuildOnBrokenLinks,
-        ),
-      );
-    }
+    pipeline.finish(
+      BrokenLinkFinderFinisher(
+        shouldRunLinkVerification: true == shouldRunLinkVerification ||
+            (shouldRunLinkVerification == null && context.buildMode == StaticShockBuildMode.production),
+        reportBrokenLinksAsErrors: failBuildOnBrokenLinks,
+      ),
+    );
+
+    pipeline.finish(
+      FindMissingPagesFinisher(
+        cache: pluginCache,
+        reportMissingPagesAtErrorLevel: reportMissingPagesAtErrorLevel ??
+            switch (context.buildMode) {
+              StaticShockBuildMode.production => StaticShockErrorLevel.error,
+              StaticShockBuildMode.dev => StaticShockErrorLevel.warning,
+            },
+        cacheUpdatePolicy: pageManifestUpdatePolicy ??
+            switch (context.buildMode) {
+              StaticShockBuildMode.production => PageManifestUpdatePolicy.forceUpdate,
+              StaticShockBuildMode.dev => PageManifestUpdatePolicy.noUpdate,
+            },
+        includeDraftPages: includeDraftPagesInPageManifest,
+      ),
+    );
   }
 }
 
 class BrokenLinkFinderFinisher implements Finisher {
   const BrokenLinkFinderFinisher({
+    required this.shouldRunLinkVerification,
     this.reportBrokenLinksAsErrors = false,
   });
 
+  /// Whether to run link verification or not.
+  final bool shouldRunLinkVerification;
+
   /// Whether to report broken links as errors, rather than warnings.
   ///
   /// Errors are typically handled as build failures, whereas warnings still
@@ -60,6 +104,11 @@ class BrokenLinkFinderFinisher implements Finisher {
 
   @override
   Future<void> execute(StaticShockPipelineContext context) async {
+    if (!shouldRunLinkVerification) {
+      context.log.info("Skipping link verification for this build");
+      return;
+    }
+
     context.log.info("Validating all external and internal links in the built website.");
 
     final linkFinder = HtmlLinkFinder();
@@ -123,6 +172,94 @@ class BrokenLinkFinderFinisher implements Finisher {
   }
 }
 
+/// [Finisher] that finds every page in this build, ensure that no pages were lost from
+/// the last build, and saves an updated page manifest for future build checks.
+class FindMissingPagesFinisher implements Finisher {
+  static const pageManifestCacheKey = "pages-manifest.json";
+
+  FindMissingPagesFinisher({
+    required this.cache,
+    this.cacheUpdatePolicy = PageManifestUpdatePolicy.noUpdate,
+    this.reportMissingPagesAtErrorLevel = StaticShockErrorLevel.warning,
+    this.includeDraftPages = false,
+  });
+
+  /// The cache where the previous list of pages was stored, and where the
+  /// new list will be stored.
+  final StaticShockCache cache;
+
+  /// The policy that determines whether the page cache is updated during this build.
+  final PageManifestUpdatePolicy cacheUpdatePolicy;
+
+  /// Whether the website build should fail if any of the pages from the existing cached list
+  /// no longer exist in the current website build.
+  final StaticShockErrorLevel reportMissingPagesAtErrorLevel;
+
+  /// Whether to save page URLs for draft (not yet published) pages.
+  final bool includeDraftPages;
+
+  @override
+  Future<void> execute(StaticShockPipelineContext context) async {
+    context.log.info("Checking for any missing pages from the previous build");
+
+    final currentBuildPages = _collectAllPagePaths(context);
+    context.log.detail("Found ${currentBuildPages.length} pages in the current build.");
+
+    final previousBuildPagesJson = await cache.loadJsonList(pageManifestCacheKey);
+    final previousBuildPages = previousBuildPagesJson?.cast<String>().toSet();
+
+    final missingPages = <String>{};
+    if (previousBuildPages != null) {
+      // We have a previous build to compare to. Find any missing pages.
+      context.log.detail("Found ${previousBuildPages.length} pages in the previous build.");
+
+      for (final existingPage in previousBuildPages) {
+        if (!currentBuildPages.contains(existingPage)) {
+          missingPages.add(existingPage);
+        }
+      }
+
+      context.log.detail("Found ${missingPages.length} missing pages");
+    } else {
+      context.log.detail("No existing page manifest was found. Nothing to compare to.");
+    }
+
+    if (cacheUpdatePolicy == PageManifestUpdatePolicy.forceUpdate ||
+        (cacheUpdatePolicy == PageManifestUpdatePolicy.updateIfNoMissingPages && missingPages.isEmpty)) {
+      context.log.detail("Updating the page manifest based on the pages in the current build.");
+      await cache.putJsonList(pageManifestCacheKey, currentBuildPages.toList(growable: false));
+    } else {
+      context.log.detail("Not updating page cache in this build.");
+    }
+
+    // Log missing pages (and maybe) fail the build.
+    if (missingPages.isNotEmpty) {
+      final message = [
+        "Found ${missingPages.length} missing pages in the current build.",
+        ...missingPages.map((path) => " • '$path'"),
+      ].join("\n");
+
+      context.errorLog.log(reportMissingPagesAtErrorLevel, message);
+    }
+  }
+
+  Set<String> _collectAllPagePaths(StaticShockPipelineContext context) {
+    return context.pagesIndex.pages //
+        .where((page) => includeDraftPages ? true : !page.tags.contains("drafting"))
+        // Note: These are page paths, not links, so we expect every page path
+        // to define itself with an absolute path - no relative paths. Therefore,
+        // we shouldn't need to do any relative path resolution here.
+        .map((page) => page.pagePath!)
+        .toSet();
+  }
+}
+
+enum PageManifestUpdatePolicy {
+  noUpdate,
+  updateIfNoMissingPages,
+  forceUpdate;
+}
+
 extension NormalizeUris on Iterable<Uri> {
   Iterable<Uri> normalizeUris() => map(_normalizeUri);
 
diff --git a/packages/static_shock/lib/src/static_shock.dart b/packages/static_shock/lib/src/static_shock.dart
index 1de7aae..80d3589 100644
--- a/packages/static_shock/lib/src/static_shock.dart
+++ b/packages/static_shock/lib/src/static_shock.dart
@@ -90,6 +90,16 @@ class StaticShock implements StaticShockPipeline {
       return;
     }
 
+    if (arguments.contains("--production")) {
+      _buildMode = StaticShockBuildMode.production;
+      return;
+    }
+
+    if (arguments.contains("--dev")) {
+      _buildMode = StaticShockBuildMode.dev;
+      return;
+    }
+
     final buildModeArgs = arguments.where((arg) => arg.startsWith("--build-mode"));
     if (buildModeArgs.isEmpty) {
       // No explicit build mode.
diff --git a/packages/static_shock_cli/lib/src/commands/build_command.dart b/packages/static_shock_cli/lib/src/commands/build_command.dart
index e2a4d20..1a57041 100644
--- a/packages/static_shock_cli/lib/src/commands/build_command.dart
+++ b/packages/static_shock_cli/lib/src/commands/build_command.dart
@@ -4,7 +4,11 @@ import 'package:static_shock_cli/src/website_builder.dart';
 
 /// CLI [Command] that builds a Static Shock website from source files.
 class BuildCommand extends Command {
-  BuildCommand(this.log);
+  BuildCommand(this.log) {
+    argParser //
+      ..addFlag("production", help: "Build in production mode.")
+      ..addFlag("dev", help: "Build in dev mode");
+  }
 
   final Logger log;
 
@@ -25,7 +29,13 @@ class BuildCommand extends Command {
     }
 
     await buildWebsite(
-      appArguments: argResults?.rest ?? [],
+      appArguments: [
+        if (argResults!.flag("production")) //
+          "--production",
+        if (argResults!.flag("dev")) //
+          "--dev",
+        ...argResults!.rest,
+      ],
       log: log,
     );
   }
diff --git a/packages/static_shock_docs/source/guides/_data.yaml b/packages/static_shock_docs/source/guides/_data.yaml
index ff4672e..19dd226 100644
--- a/packages/static_shock_docs/source/guides/_data.yaml
+++ b/packages/static_shock_docs/source/guides/_data.yaml
@@ -22,6 +22,8 @@ docs_menu:
           id: deploy-to-github-pages
         - title: Validate Links
           id: validate-links
+        - title: Find Missing Pages
+          id: find-missing-pages
     - title: Website
       id: website
       items:
diff --git a/packages/static_shock_docs/source/guides/find-missing-pages.md b/packages/static_shock_docs/source/guides/find-missing-pages.md
new file mode 100644
index 0000000..0b99eb6
--- /dev/null
+++ b/packages/static_shock_docs/source/guides/find-missing-pages.md
@@ -0,0 +1,45 @@
+---
+title: Find Missing Pages
+---
+When editing a static site, it's easy to forget that every page that you've deployed
+now has a public URL that others might depend upon. Users might add a URL to their
+bookmarks, or they might share the URL on social media. Therefore, it's typically
+recommended that you never remove an existing URL - you should only redirect it.
+
+The first step in making sure you don't break a public URL is to make sure that
+URL doesn't disappear from your website build. Another way to say the same thing
+is that you need to make sure pages don't disappear from one deployment to another.
+
+The `LinksPlugin` includes support for watching your pages from one deployment to
+the next, and it can alert you when pages disappear.
+
+```dart
+final site = StaticShock(...)
+  ..plugin(const LinksPlugin());
+```
+
+By default, the `LinksPlugin` updates its list of pages when you run a production build,
+but not when you run a development build. This is because what you're guarding against
+is the loss of published pages. Similarly, by default, the plugin ignores all pages in
+draft mode. 
+
+You can override these defaults.
+
+```dart
+final site = StaticShock(...)
+  ..plugin(const LinksPlugin(
+    pageManifestUpdatePolicy: PageManifestUpdatePolicy.forceUpdate,
+    includeDraftPagesInPageManifest: true,
+  ));
+```
+
+By default, the `LinksPlugin` reports missing pages as warnings in dev mode, and as errors
+in production mode. However, you can override the default behavior to report them however
+you'd like.
+
+```dart
+final site = StaticShock(...)
+  ..plugin(const LinksPlugin(
+    reportMissingPagesAtErrorLevel: StaticShockErrorLevel.warning,
+  ));
+```
\ No newline at end of file


Test patch:
diff --git a/packages/static_shock/test/plugins/links_test.dart b/packages/static_shock/test/plugins/links_test.dart
index 4973d99..f83bf67 100644
--- a/packages/static_shock/test/plugins/links_test.dart
+++ b/packages/static_shock/test/plugins/links_test.dart
@@ -9,7 +9,7 @@ void main() {
       final errorLog = ErrorLog();
       final context = _createFakeSite(Directory("test/fake/"), errorLog);
 
-      await BrokenLinkFinderFinisher().execute(context);
+      await BrokenLinkFinderFinisher(shouldRunLinkVerification: true).execute(context);
 
       expect(errorLog.hasWarnings, isTrue);
       expect(errorLog.warnings, [
@@ -32,7 +32,7 @@ void main() {
         "baseUrl": "https://staticshock.io",
       });
 
-      await BrokenLinkFinderFinisher().execute(context);
+      await BrokenLinkFinderFinisher(shouldRunLinkVerification: true).execute(context);
 
       expect(errorLog.hasWarnings, isTrue);
       expect(errorLog.warnings, [