Revert "feat: Add screenshot storage support to broken link checker (#123)"

This reverts commit cd34e35.

Author: adamfweidman

package-lock.json

@@ -27,24 +27,20 @@
     "@types/chai": "^4.3.4",
     "@types/express": "^4.17.17",
     "@types/node": "^18.15.10",
-    "@types/proxyquire": "^1.3.31",
     "@types/sinon": "^10.0.16",
     "@types/supertest": "^2.0.12",
     "chai": "^4.3.7",
     "chai-exclude": "^2.1.0",
     "express": "^4.18.2",
-    "proxyquire": "^2.1.3",
-    "node-mocks-http": "^1.13.0",
-    "sinon": "^16.1.1",
+    "sinon": "^15.2.0",
     "supertest": "^6.3.3",
     "synthetics-sdk-broken-links": "file:./"
   },
   "engines": {
     "node": ">=18"
   },
   "dependencies": {
-    "@google-cloud/storage": "^7.7.0",
-    "@google-cloud/synthetics-sdk-api": "^0.6.0",
+    "@google-cloud/synthetics-sdk-api": "^0.5.1",
     "puppeteer": "21.3.6"
   }
 }

packages/synthetics-sdk-broken-links/package.json

@@ -12,13 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Internal Project Files
+import puppeteer, { Browser, Page } from 'puppeteer';
 import {
-  BaseError,
   BrokenLinksResultV1_BrokenLinkCheckerOptions,
   BrokenLinksResultV1_SyntheticLinkResult,
-  getRuntimeMetadata,
   instantiateMetadata,
+  getRuntimeMetadata,
   SyntheticResult,
 } from '@google-cloud/synthetics-sdk-api';
 import {
@@ -33,19 +32,10 @@ import {
   checkLinks,
   closeBrowser,
   closePagePool,
-  openNewPage,
   retrieveLinksFromPage,
+  openNewPage,
 } from './navigation_func';
-import { processOptions } from './options_func';
-import {
-  createStorageClientIfStorageSelected,
-  getOrCreateStorageBucket,
-  StorageParameters,
-} from './storage_func';
-
-// External Dependencies
-import { Bucket } from '@google-cloud/storage';
-import puppeteer, { Browser, Page } from 'puppeteer';
+import { setDefaultOptions, validateInputOptions } from './options_func';
 
 export interface BrokenLinkCheckerOptions {
   origin_uri: string;
@@ -58,7 +48,6 @@ export interface BrokenLinkCheckerOptions {
   wait_for_selector?: string;
   per_link_options?: { [key: string]: PerLinkOption };
   total_synthetic_timeout_millis?: number;
-  screenshot_options?: ScreenshotOptions;
 }
 
 export interface PerLinkOption {
@@ -81,17 +70,6 @@ export enum StatusClass {
   STATUS_CLASS_ANY = 'STATUS_CLASS_ANY',
 }
 
-export interface ScreenshotOptions {
-  storage_location?: string;
-  capture_condition?: CaptureCondition;
-}
-
-export enum CaptureCondition {
-  NONE = 'NONE',
-  FAILING = 'FAILING',
-  ALL = 'ALL',
-}
-
 let synthetics_sdk_broken_links_package;
 try {
   synthetics_sdk_broken_links_package = require('../package.json');
@@ -101,11 +79,7 @@ try {
 instantiateMetadata(synthetics_sdk_broken_links_package);
 
 export async function runBrokenLinks(
-  inputOptions: BrokenLinkCheckerOptions,
-  args: {
-    executionId: string | undefined;
-    checkId: string | undefined;
-  }
+  inputOptions: BrokenLinkCheckerOptions
 ): Promise<SyntheticResult> {
   // init
   const startTime = new Date().toISOString();
@@ -122,30 +96,6 @@ export async function runBrokenLinks(
     const [timeLimitPromise, timeLimitTimeout, timeLimitresolver] =
       getTimeLimitPromise(startTime, adjusted_synthetic_timeout_millis);
 
-    const errors: BaseError[] = [];
-
-    // Initialize Storage Client with Error Handling. Set to `null` if
-    // capture_condition is 'None'
-    const storageClient = createStorageClientIfStorageSelected(
-      errors,
-      options.screenshot_options!.capture_condition
-    );
-
-    // // Bucket Validation
-    const bucket: Bucket | null = await getOrCreateStorageBucket(
-      storageClient,
-      options.screenshot_options!.storage_location,
-      errors
-    );
-
-    const storageParams: StorageParameters = {
-      storageClient: storageClient,
-      bucket: bucket,
-      checkId: args.checkId || '_',
-      executionId: args.executionId || '_',
-      screenshotNumber: 1,
-    };
-
     const followed_links: BrokenLinksResultV1_SyntheticLinkResult[] = [];
 
     const checkLinksPromise = async () => {
@@ -159,8 +109,7 @@ export async function runBrokenLinks(
           originPage,
           options,
           startTime,
-          adjusted_synthetic_timeout_millis,
-          storageParams
+          adjusted_synthetic_timeout_millis
         )
       );
 
@@ -182,8 +131,7 @@ export async function runBrokenLinks(
           linksToFollow,
           options,
           startTime,
-          adjusted_synthetic_timeout_millis,
-          storageParams
+          adjusted_synthetic_timeout_millis
         ))
       );
       return true;
@@ -201,9 +149,7 @@ export async function runBrokenLinks(
       startTime,
       runtime_metadata,
       options,
-      followed_links,
-      storageParams,
-      errors
+      followed_links
     );
   } catch (err) {
     const errorMessage =
@@ -230,8 +176,7 @@ async function checkOriginLink(
   originPage: Page,
   options: BrokenLinksResultV1_BrokenLinkCheckerOptions,
   startTime: string,
-  adjusted_synthetic_timeout_millis: number,
-  storageParams: StorageParameters
+  adjusted_synthetic_timeout_millis: number
 ): Promise<BrokenLinksResultV1_SyntheticLinkResult> {
   let originLinkResult: BrokenLinksResultV1_SyntheticLinkResult;
 
@@ -248,7 +193,6 @@ async function checkOriginLink(
       originPage,
       { target_uri: options.origin_uri, anchor_text: '', html_element: '' },
       options,
-      storageParams,
       true
     );
 
@@ -319,3 +263,16 @@ async function scrapeLinks(
     options.link_order
   );
 }
+
+/**
+ * Validates input options and sets defaults in `options`.
+ *
+ * @param inputOptions - The input options for the broken link checker.
+ * @returns The processed broken link checker options.
+ */
+function processOptions(
+  inputOptions: BrokenLinkCheckerOptions
+): BrokenLinksResultV1_BrokenLinkCheckerOptions {
+  const validOptions = validateInputOptions(inputOptions);
+  return setDefaultOptions(validOptions);
+}

packages/synthetics-sdk-broken-links/src/broken_links.ts

@@ -12,14 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Standard Libraries
-import { Request, Response } from 'express';
-
-// Internal Project Files
 import { runBrokenLinks, BrokenLinkCheckerOptions } from './broken_links';
-
-const syntheticExecutionIdHeader = 'Synthetic-Execution-Id';
-const checkIdHeader = 'Check-Id';
+import { Request, Response } from 'express';
 
 /**
  * Middleware for easy invocation of SyntheticSDK broken links, and may be used to
@@ -32,10 +26,5 @@ const checkIdHeader = 'Check-Id';
 export function runBrokenLinksHandler(options: BrokenLinkCheckerOptions) {
   // eslint-disable-next-line  @typescript-eslint/no-explicit-any
   return async (req: Request, res: Response): Promise<any> =>
-    res.send(
-      await runBrokenLinks(options, {
-        executionId: req.get(syntheticExecutionIdHeader),
-        checkId: req.get(checkIdHeader),
-      })
-    );
+    res.send(await runBrokenLinks(options));
 }

packages/synthetics-sdk-broken-links/src/handlers.ts

@@ -13,11 +13,11 @@
 // limitations under the License.
 
 export {
+  runBrokenLinks,
   BrokenLinkCheckerOptions,
-  LinkOrder,
   PerLinkOption,
-  runBrokenLinks,
   StatusClass,
+  LinkOrder,
 } from './broken_links';
 export * from './handlers';
 export * from '@google-cloud/synthetics-sdk-api';

packages/synthetics-sdk-broken-links/src/index.ts

@@ -12,16 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Standard Libraries
-import * as path from 'path';
-
-// Internal Project Files
+import { HTTPResponse } from 'puppeteer';
 import {
-  BaseError,
   BrokenLinksResultV1,
   BrokenLinksResultV1_BrokenLinkCheckerOptions,
   BrokenLinksResultV1_BrokenLinkCheckerOptions_LinkOrder,
-  BrokenLinksResultV1_BrokenLinkCheckerOptions_ScreenshotOptions_CaptureCondition as ApiCaptureCondition,
   BrokenLinksResultV1_SyntheticLinkResult,
   GenericResultV1,
   getRuntimeMetadata,
@@ -30,10 +25,6 @@ import {
   SyntheticResult,
 } from '@google-cloud/synthetics-sdk-api';
 
-// External Dependencies
-import { HTTPResponse } from 'puppeteer';
-import { StorageParameters } from './storage_func';
-
 /**
  * Represents an intermediate link with its properties.
  */
@@ -162,8 +153,6 @@ function parseFollowedLinks(
     options: {} as BrokenLinksResultV1_BrokenLinkCheckerOptions,
     origin_link_result: {} as BrokenLinksResultV1_SyntheticLinkResult,
     followed_link_results: [],
-    execution_data_storage_path: '',
-    errors: [],
   };
 
   for (const link of followed_links) {
@@ -227,21 +216,12 @@ export function createSyntheticResult(
   start_time: string,
   runtime_metadata: { [key: string]: string },
   options: BrokenLinksResultV1_BrokenLinkCheckerOptions,
-  followed_links: BrokenLinksResultV1_SyntheticLinkResult[],
-  storageParams: StorageParameters,
-  errors: BaseError[]
+  followed_links: BrokenLinksResultV1_SyntheticLinkResult[]
 ): SyntheticResult {
   // Create BrokenLinksResultV1 by parsing followed links and setting options
   const broken_links_result: BrokenLinksResultV1 =
     parseFollowedLinks(followed_links);
   broken_links_result.options = options;
-  broken_links_result.errors = errors;
-  broken_links_result.execution_data_storage_path = storageParams.bucket
-    ? 'gs://' +
-      storageParams.bucket.name +
-      '/' +
-      getStoragePathToExecution(storageParams, options)
-    : '';
 
   // Create SyntheticResult object
   const synthetic_result: SyntheticResult = {
@@ -284,89 +264,6 @@ export function shuffleAndTruncate(
   return linksToFollow.slice(0, link_limit! - 1);
 }
 
-/**
- * Determines whether a screenshot should be taken based on screenshot options and link result.
- *
- * @param options - BrokenLinksResultV1_BrokenLinkCheckerOptions
- * @param passed -  boolean indicating whether the link navigation succeeded
- * @returns true if a screenshot should be taken, false otherwise
- */
-export function shouldTakeScreenshot(
-  options: BrokenLinksResultV1_BrokenLinkCheckerOptions,
-  passed: boolean
-): boolean {
-  return (
-    options.screenshot_options!.capture_condition === ApiCaptureCondition.ALL ||
-    (options.screenshot_options!.capture_condition ===
-      ApiCaptureCondition.FAILING &&
-      !passed)
-  );
-}
-
-/**
-
- * Sanitizes an object name string for safe use, ensuring compliance with
- * naming restrictions.
- *
- * @param {string} inputString - The original object name string.
- * @returns {string} The sanitized object name.
- *
- * **Sanitization Rules:**
- * * Removes control characters ([\u007F-\u009F]).
- * * Removes disallowed characters (#, [, ], *, ?, ", <, >, |, /).
- * * Replaces the forbidden prefix ".well-known/acme-challenge/" with an underscore.
- * * Replaces standalone occurrences of "." or ".." with an underscore.
- */
-export function sanitizeObjectName(
-  inputString: string | null | undefined
-): string {
-  if (!inputString || inputString === '.' || inputString === '..') return '_';
-
-  // Regular expressions for:
-  /*eslint no-useless-escape: "off"*/
-  const invalidCharactersRegex = /[\r\n\u007F-\u009F#\[\]*?:"<>|/]/g; // Control characters, special characters, path separator
-  const wellKnownPrefixRegex = /^\.well-known\/acme-challenge\//;
-
-  // Core sanitization:
-  return inputString
-    .replace(wellKnownPrefixRegex, '_') // Replace forbidden prefix
-    .replace(invalidCharactersRegex, '_') // replace invalid characters
-    .trim() // Clean up any leading/trailing spaces
-    .replace(/\s+/g, '_'); // Replace one or more spaces with underscores
-}
-
-export function getStoragePathToExecution(
-  storageParams: StorageParameters,
-  options: BrokenLinksResultV1_BrokenLinkCheckerOptions
-) {
-  try {
-    const storageLocation = options.screenshot_options!.storage_location;
-    let writeDestination = '';
-
-    // extract folder name for a given storage location. If there is no '/'
-    // present then the storageLocation is just a folder
-    const firstSlashIndex = storageLocation.indexOf('/');
-    if (firstSlashIndex !== -1) {
-      writeDestination = storageLocation.substring(firstSlashIndex + 1);
-    }
-
-    // Ensure writeDestination ends with a slash for proper path joining
-    if (writeDestination && !writeDestination.endsWith('/')) {
-      writeDestination += '/';
-    }
-
-    writeDestination = path.join(
-      writeDestination,
-      storageParams.checkId,
-      storageParams.executionId
-    );
-
-    return writeDestination;
-  } catch (err) {
-    return '';
-  }
-}
-
 export function getTimeLimitPromise(
   startTime: string,
   totalTimeoutMillis: number,