Merge pull request #13 from applandinc/refactor/options-objects

feat: Scanner are classes with named fields

src/sampleConfig/default.ts

@@ -10,19 +10,16 @@ import slowQuery from '../scanner/slowQuery';
 import validateBeforeSave from '../scanner/validateBeforeSave';
 
 const assertions: Assertion[] = [
-  slowHttpServerRequest(),
-  slowQuery(0.05),
-  queryFromView(),
+  slowHttpServerRequest.scanner(),
+  slowQuery.scanner(new slowQuery.Options(0.05)),
+  queryFromView.scanner(),
   missingContentType(),
-  missingAuthentication(),
+  missingAuthentication.scanner(),
   validateBeforeSave(),
   leafExpected('http_client_request'),
   leafExpected('sql_query'),
   leafExpected('function', (e: Event) => e.codeObject.labels.has('logging')),
-  nPlusOneQuery({
-    limit: { warning: 3, error: 10 },
-    whitelist: [`SELECT * FROM "users" WHERE "id" = ?`],
-  }),
+  nPlusOneQuery.scanner(new nPlusOneQuery.Options(3, [`SELECT * FROM "users" WHERE "id" = ?`])),
 ];
 
 export default function (): Assertion[] {

src/sampleConfig/forem.ts

@@ -24,11 +24,11 @@ const slowFunction = function (options: SlowFunctionOptions): Assertion {
 };
 
 const assertions: Assertion[] = [
-  slowHttpServerRequest(),
-  slowQuery(0.05),
+  slowHttpServerRequest.scanner(),
+  slowQuery.scanner(new slowQuery.Options(0.05)),
   slowFunction({ timeAllowed: 0.05, fn: 'app/models/Article#comments_blob' }),
   missingContentType(),
-  missingAuthentication(),
+  missingAuthentication.scanner(),
   leafExpected('http_client_request'),
   leafExpected('sql_query'),
 ];

src/sampleConfig/railsSampleApp6thEd.ts

@@ -7,10 +7,10 @@ import slowQuery from '../scanner/slowQuery';
 import updateInGetRequest from '../scanner/updateInGetRequest';
 
 const assertions: Assertion[] = [
-  slowHttpServerRequest(),
-  slowQuery(0.05),
+  slowHttpServerRequest.scanner(),
+  slowQuery.scanner(new slowQuery.Options(0.05)),
   missingContentType(),
-  missingAuthentication(),
+  missingAuthentication.scanner(),
   leafExpected('http_client_request'),
   leafExpected('sql_query'),
   updateInGetRequest.scanner(),

src/scanner/missingAuthentication.ts

@@ -22,7 +22,11 @@ const authenticatedBy = (iterator: Iterator<EventNavigator>): boolean => {
   return false;
 };
 
-export default function (routes: RegExp[] = [/.*/], contentTypes: RegExp[] = [/.*/]): Assertion {
+class Options {
+  constructor(public routes: RegExp[] = [/.*/], public contentTypes: RegExp[] = [/.*/]) {}
+}
+
+function scanner(options: Options = new Options()): Assertion {
   return Assertion.assert(
     'missing-authentication',
     'Unauthenticated HTTP server requests',
@@ -35,11 +39,13 @@ export default function (routes: RegExp[] = [/.*/], contentTypes: RegExp[] = [/.
           e.httpServerResponse !== undefined &&
           e.httpServerResponse!.status_code < 300 &&
           contentType(e) !== undefined &&
-          routes.some((pattern) => pattern.test(e.route!)) &&
-          contentTypes.some((pattern) => pattern.test(contentType(e)!))
+          options.routes.some((pattern) => pattern.test(e.route!)) &&
+          options.contentTypes.some((pattern) => pattern.test(contentType(e)!))
         );
       };
       assertion.description = `HTTP server request must be authenticated`;
     }
   );
 }
+
+export default { Options, scanner };

src/scanner/nPlusOneQuery.ts

@@ -14,16 +14,15 @@ function sqlNormalized(query: SqlQuery): string {
   return obfuscate(query.sql, query.database_type);
 }
 
-class Limits {
-  constructor(public warning: number = 3, public error: number = 10) {}
-}
-
 class Options {
-  limit: Limits = new Limits();
-  whitelist: string[] = [];
+  /**
+  TODO: Warn on warningLimit, error on errorLimit.
+  errorLimit = 10;
+   */
+  constructor(public warningLimit = 3, public whitelist: string[] = []) {}
 }
 
-export default function (options: Options = new Options()): Assertion {
+function scanner(options: Options = new Options()): Assertion {
   const uniqueSQL = new Set<string>();
   const matchedSQL = new Set<string>();
 
@@ -67,7 +66,7 @@ export default function (options: Options = new Options()): Assertion {
     'n-plus-one-query',
     'Duplicate SQL queries',
     'sql_query',
-    (event: Event, appMap: AppMap) => findDuplicates(event, appMap) < options.limit.warning,
+    (event: Event, appMap: AppMap) => findDuplicates(event, appMap) < options.warningLimit,
     (assertion: Assertion): void => {
       assertion.where = (e: Event, appMap: AppMap) => {
         if (getSqlLabelFromString(e.sqlQuery!) !== 'SQL Select') {
@@ -92,3 +91,5 @@ export default function (options: Options = new Options()): Assertion {
     }
   );
 }
+
+export default { Options, scanner };

src/scanner/queryFromConstraint.ts

@@ -3,18 +3,26 @@ import Assertion from '../assertion';
 
 const WHITELIST = [/BEGIN/, /COMMIT/, /ROLLBACK/, /RELEASE/, /SAVEPOINT/];
 
-export default function (parentPackages: string[], whitelist: RegExp[] = []): Assertion {
+class Options {
+  constructor(public parentPackages: string[] = [], public whitelist: RegExp[] = []) {}
+}
+
+function scanner(options: Options): Assertion {
   return Assertion.assert(
     'query-from-invalid-package',
     'Queries from invalid packages',
     'event',
-    (e: Event) => parentPackages.includes(e.parent!.codeObject.packageOf),
+    (e: Event) => options.parentPackages.includes(e.parent!.codeObject.packageOf),
     (assertion: Assertion): void => {
       assertion.where = (e: Event) =>
         e.sqlQuery !== undefined &&
         e.parent !== undefined &&
-        !whitelist.concat(WHITELIST).some((pattern) => pattern.test(e.sqlQuery!));
-      assertion.description = `Query must be invoked from one of (${parentPackages.join(',')})`;
+        !options.whitelist.concat(WHITELIST).some((pattern) => pattern.test(e.sqlQuery!));
+      assertion.description = `Query must be invoked from one of (${options.parentPackages.join(
+        ','
+      )})`;
     }
   );
 }
+
+export default { Options, scanner };

src/scanner/queryFromView.ts

@@ -1,14 +1,21 @@
 import { Event } from '@appland/models';
 import Assertion from '../assertion';
 
-export default function (forbiddenLabel = 'mvc.template'): Assertion {
+class Options {
+  constructor(public forbiddenLabel = 'mvc.template') {}
+}
+
+function scanner(options: Options = new Options()): Assertion {
   return Assertion.assert(
     'query-from-view',
     'Queries from view',
     'sql_query',
-    (e: Event) => e.ancestors().every((e: Event) => !e.codeObject.labels.has(forbiddenLabel)),
+    (e: Event) =>
+      e.ancestors().every((e: Event) => !e.codeObject.labels.has(options.forbiddenLabel)),
     (assertion: Assertion): void => {
-      assertion.description = `SQL query from ${forbiddenLabel}`;
+      assertion.description = `SQL query from ${options.forbiddenLabel}`;
     }
   );
 }
+
+export default { Options, scanner };

src/scanner/slowHttpServerRequest.ts

@@ -1,15 +1,21 @@
 import { Event } from '@appland/models';
 import Assertion from '../assertion';
 
-export default function (timeAllowed = 1): Assertion {
+class Options {
+  constructor(public timeAllowed = 1) {}
+}
+
+function scanner(options: Options = new Options()): Assertion {
   return Assertion.assert(
     'slow-http-server-request',
     'Slow HTTP server requests',
     'http_server_request',
-    (e: Event) => e.elapsedTime! < timeAllowed,
+    (e: Event) => e.elapsedTime! < options.timeAllowed,
     (assertion: Assertion): void => {
       assertion.where = (e: Event) => e.elapsedTime !== undefined;
-      assertion.description = `Slow HTTP server request (> ${timeAllowed * 1000}ms)`;
+      assertion.description = `Slow HTTP server request (> ${options.timeAllowed * 1000}ms)`;
     }
   );
 }
+
+export default { Options, scanner };

src/scanner/slowQuery.ts

@@ -1,22 +1,28 @@
 import { Event } from '@appland/models';
 import Assertion from '../assertion';
 
-export default function (
-  timeAllowed = 1,
-  queryInclude = [/SELECT/],
-  queryExclude = [/pg_advisory_xact_lock/]
-): Assertion {
+class Options {
+  constructor(
+    public timeAllowed = 1,
+    public queryInclude = [/SELECT/i],
+    public queryExclude = [/pg_advisory_xact_lock/]
+  ) {}
+}
+
+function scanner(options: Options = new Options()): Assertion {
   return Assertion.assert(
     'slow-query',
     'Slow SQL queries',
     'sql_query',
-    (e: Event) => e.elapsedTime! < timeAllowed,
+    (e: Event) => e.elapsedTime! < options.timeAllowed,
     (assertion: Assertion): void => {
       assertion.where = (e: Event) =>
         e.elapsedTime !== undefined &&
-        queryInclude.some((pattern) => e.sqlQuery && e.sqlQuery.match(pattern)) &&
-        !queryExclude.some((pattern) => e.sqlQuery && e.sqlQuery.match(pattern));
-      assertion.description = `Slow SQL query (> ${timeAllowed * 1000}ms)`;
+        options.queryInclude.some((pattern) => e.sqlQuery && e.sqlQuery.match(pattern)) &&
+        !options.queryExclude.some((pattern) => e.sqlQuery && e.sqlQuery.match(pattern));
+      assertion.description = `Slow SQL query (> ${options.timeAllowed * 1000}ms)`;
     }
   );
 }
+
+export default { Options, scanner };

src/scanner/updateInGetRequest.ts

@@ -1,16 +1,14 @@
 import { Event } from '@appland/models';
 import Assertion from '../assertion';
 
-class UpdateInGetRequestOptions {
+class Options {
   constructor(
     public queryInclude: RegExp[] = [/INSERT/i, /UPDATE/i],
     public queryExclude: RegExp[] = []
   ) {}
 }
 
-function updateInGetRequest(
-  options: UpdateInGetRequestOptions = new UpdateInGetRequestOptions()
-): Assertion {
+function scanner(options: Options = new Options()): Assertion {
   return Assertion.assert(
     'update-in-get-request',
     'Data update performed in GET or HEAD request',
@@ -34,4 +32,4 @@ function updateInGetRequest(
   );
 }
 
-export default { options: UpdateInGetRequestOptions, scanner: updateInGetRequest };
+export default { Options, scanner };