Add explicit type information to checks

Types can be used to:
* Determine which check logic to run
* Determine the correct persistence option to use
* Choose correct ser/deser logic for the check
* Determine correct components to render on the frontend

Modify router API to be post for better usability. The API might end up doing writes if caching etc. is added in later as well.

Modify CheckRegistry & FactChecker to return persisted check when it is registered.

Signed-off-by: Jussi Hallila <jussi@hallila.com>

plugins/tech-insights-backend-module-jsonfc/README.md

@@ -52,10 +52,12 @@ Checks for this FactChecker are constructed as `json-rules-engine` compatible JS
 
 ```ts
 import { TechInsightJsonRuleCheck } from '../types';
+import { JSON_RULE_ENGINE_CHECK_TYPE } from '../constants';
 
 export const exampleCheck: TechInsightJsonRuleCheck = {
   id: 'demodatacheck', // Unique identifier of this check
   name: 'demodatacheck', // A human readable name of this check to be displayed in the UI
+  type: JSON_RULE_ENGINE_CHECK_TYPE, // Type identifier of the check. Used to run logic against, determine persistence option to use and render correct components on the UI
   description: 'A fact check for demoing purposes', // A description to be displayed in the UI
   factRefs: ['demo-poc.factretriever'], // References to fact containers that this check uses. See documentation on FactRetrievers for more information on these
   rule: {

plugins/tech-insights-backend-module-jsonfc/api-report.md

@@ -33,6 +33,9 @@ export interface DynamicFact<T = unknown> {
   options?: FactOptions;
 }
 
+// @public (undocumented)
+export const JSON_RULE_ENGINE_CHECK_TYPE = 'json-rules-engine';
+
 // @public (undocumented)
 export interface JsonRuleBooleanCheckResult extends BooleanCheckResult {
   // (undocumented)
@@ -60,7 +63,7 @@ export class JsonRulesEngineFactChecker
     checkRegistry,
   }: JsonRulesEngineFactCheckerOptions);
   // (undocumented)
-  addCheck(check: TechInsightJsonRuleCheck): Promise<boolean>;
+  addCheck(check: TechInsightJsonRuleCheck): Promise<TechInsightJsonRuleCheck>;
   // (undocumented)
   getChecks(): Promise<TechInsightJsonRuleCheck[]>;
   // (undocumented)

plugins/tech-insights-backend-module-jsonfc/src/constants.ts

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * @public
+ */
+export const JSON_RULE_ENGINE_CHECK_TYPE = 'json-rules-engine';

plugins/tech-insights-backend-module-jsonfc/src/index.ts

@@ -17,6 +17,7 @@ export {
   JsonRulesEngineFactCheckerFactory,
   JsonRulesEngineFactChecker,
 } from './service/JsonRulesEngineFactChecker';
+export { JSON_RULE_ENGINE_CHECK_TYPE } from './constants';
 export type {
   JsonRulesEngineFactCheckerFactoryOptions,
   JsonRulesEngineFactCheckerOptions,

plugins/tech-insights-backend-module-jsonfc/src/service/CheckRegistry.ts

@@ -31,13 +31,14 @@ export class DefaultCheckRegistry<CheckType extends TechInsightCheck>
     });
   }
 
-  async register(check: CheckType) {
+  async register(check: CheckType): Promise<CheckType> {
     if (this.checks.has(check.id)) {
       throw new ConflictError(
         `Tech insight check with id ${check.id} has already been registered`,
       );
     }
     this.checks.set(check.id, check);
+    return check;
   }
 
   async get(checkId: string): Promise<CheckType> {

plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.test.ts

@@ -18,7 +18,10 @@ import {
   TechInsightCheckRegistry,
   TechInsightsStore,
 } from '@backstage/plugin-tech-insights-common';
-import { JsonRulesEngineFactCheckerFactory } from '../index';
+import {
+  JSON_RULE_ENGINE_CHECK_TYPE,
+  JsonRulesEngineFactCheckerFactory,
+} from '../index';
 import { getVoidLogger } from '@backstage/backend-common';
 import { TechInsightJsonRuleCheck } from '../types';
 
@@ -27,6 +30,7 @@ const testChecks: Record<string, TechInsightJsonRuleCheck[]> = {
     {
       id: 'brokenTestCheck',
       name: 'brokenTestCheck',
+      type: JSON_RULE_ENGINE_CHECK_TYPE,
       description: 'Broken Check For Testing',
       factRefs: ['test-factretriever'],
       rule: {
@@ -46,6 +50,7 @@ const testChecks: Record<string, TechInsightJsonRuleCheck[]> = {
     {
       id: 'brokenTestCheck2',
       name: 'brokenTestCheck2',
+      type: JSON_RULE_ENGINE_CHECK_TYPE,
       description: 'Second Broken Check For Testing',
       factRefs: ['non-existing-factretriever'],
       rule: {
@@ -65,6 +70,7 @@ const testChecks: Record<string, TechInsightJsonRuleCheck[]> = {
     {
       id: 'simpleTestCheck',
       name: 'simpleTestCheck',
+      type: JSON_RULE_ENGINE_CHECK_TYPE,
       description: 'Simple Check For Testing',
       factRefs: ['test-factretriever'],
       rule: {
@@ -85,6 +91,7 @@ const testChecks: Record<string, TechInsightJsonRuleCheck[]> = {
     {
       id: 'simpleTestCheck2',
       name: 'simpleTestCheck2',
+      type: JSON_RULE_ENGINE_CHECK_TYPE,
       description: 'Second Simple Check For Testing',
       factRefs: ['test-factretriever'],
       rule: {

plugins/tech-insights-backend-module-jsonfc/src/service/JsonRulesEngineFactChecker.ts

@@ -29,6 +29,7 @@ import { Logger } from 'winston';
 import { pick } from 'lodash';
 import Ajv from 'ajv';
 import * as validationSchema from './validation-schema.json';
+import { JSON_RULE_ENGINE_CHECK_TYPE } from '../constants';
 
 const noopEvent = {
   type: 'noop',
@@ -113,6 +114,12 @@ export class JsonRulesEngineFactChecker
     const ajv = new Ajv({ verbose: true });
     const validator = ajv.compile(validationSchema);
     const isValidToSchema = validator(check.rule);
+    if (check.type !== JSON_RULE_ENGINE_CHECK_TYPE) {
+      this.logger.warn(
+        'Only ${JSON_RULE_ENGINE_CHECK_TYPE} checks can be registered to this fact checker',
+      );
+      return false;
+    }
     if (!isValidToSchema) {
       this.logger.warn(
         'Failed to to validate conditions against JSON schema',
@@ -146,15 +153,18 @@ export class JsonRulesEngineFactChecker
     return this.checkRegistry.list();
   }
 
-  async addCheck(check: TechInsightJsonRuleCheck): Promise<boolean> {
+  async addCheck(
+    check: TechInsightJsonRuleCheck,
+  ): Promise<TechInsightJsonRuleCheck> {
     if (!(await this.validate(check))) {
       this.logger.warn(
         `Check validation failed when adding check ${check.name} to check registry.`,
       );
-      return false;
+      throw new Error(
+        'Failed to add check to rules engine. Validation failed.',
+      );
     }
-    this.checkRegistry.register(check);
-    return true;
+    return await this.checkRegistry.register(check);
   }
 
   private retrieveFactReferences(
@@ -216,6 +226,7 @@ export class JsonRulesEngineFactChecker
   ) {
     const returnable = {
       id: techInsightCheck.id,
+      type: techInsightCheck.type,
       name: techInsightCheck.name,
       description: techInsightCheck.description,
       factRefs: techInsightCheck.factRefs,

plugins/tech-insights-backend/src/service/router.ts

@@ -83,11 +83,16 @@ export async function createRouter<
       return res.send(await factChecker.getChecks());
     });
 
-    router.get('/checks/:namespace/:kind/:name', async (req, res) => {
+    router.post('/checks/run/:namespace/:kind/:name', async (req, res) => {
       const { namespace, kind, name } = req.params;
-      const checks = req.query.checks as string[];
-      const entityTriplet = `${namespace.toLowerCase()}/${kind.toLowerCase()}/${name.toLowerCase()}`;
       try {
+        if (!('checks' in req.body)) {
+          return res.status(422).send({
+            message: 'Failed to get checks from request.',
+          });
+        }
+        const { checks }: { checks: string[] } = req.body;
+        const entityTriplet = `${namespace.toLowerCase()}/${kind.toLowerCase()}/${name.toLowerCase()}`;
         const checkResult = await factChecker.runChecks(entityTriplet, checks);
         return res.send(checkResult);
       } catch (e) {
@@ -120,7 +125,11 @@ export async function createRouter<
     const startDatetime = DateTime.fromISO(req.query.startDatetime as string);
     const endDatetime = DateTime.fromISO(req.query.endDatetime as string);
     if (!startDatetime.isValid || !endDatetime.isValid) {
-      return res.status(422).send('Failed to parse datetime from request');
+      return res.status(422).send({
+        message: 'Failed to parse datetime from request',
+        field: !startDatetime.isValid ? 'startDateTime' : 'endDateTime',
+        value: !startDatetime.isValid ? startDatetime : endDatetime,
+      });
     }
     const entityTriplet = `${namespace.toLowerCase()}/${kind.toLowerCase()}/${name.toLowerCase()}`;
     return res.send(

plugins/tech-insights-common/api-report.md

@@ -21,6 +21,7 @@ export interface CheckResponse {
   id: string;
   metadata?: Record<string, any>;
   name: string;
+  type: string;
 }
 
 // @public
@@ -34,7 +35,7 @@ export interface FactChecker<
   CheckType extends TechInsightCheck,
   CheckResultType extends CheckResult,
 > {
-  addCheck(check: CheckType): Promise<boolean>;
+  addCheck(check: CheckType): Promise<CheckType>;
   getChecks(): Promise<CheckType[]>;
   runChecks(entity: string, checks: string[]): Promise<CheckResultType[]>;
   validate(check: CheckType): Promise<boolean>;
@@ -108,14 +109,13 @@ export type FlatTechInsightFact = TechInsightFact & {
 
 // @public
 export interface TechInsightCheck {
-  // (undocumented)
   description: string;
   factRefs: string[];
   failureMetadata?: Record<string, any>;
   id: string;
-  // (undocumented)
   name: string;
   successMetadata?: Record<string, any>;
+  type: string;
 }
 
 // @public
@@ -127,7 +127,7 @@ export interface TechInsightCheckRegistry<CheckType extends TechInsightCheck> {
   // (undocumented)
   list(): Promise<CheckType[]>;
   // (undocumented)
-  register(check: CheckType): Promise<void>;
+  register(check: CheckType): Promise<CheckType>;
 }
 
 // @public

plugins/tech-insights-common/src/checks.ts

@@ -66,7 +66,7 @@ export interface FactChecker<
    * @param check - The actual check to be added.
    * @returns  - An indicator if fact was successfully added
    */
-  addCheck(check: CheckType): Promise<boolean>;
+  addCheck(check: CheckType): Promise<CheckType>;
 
   /**
    * Retrieves all available checks that can be used to run checks against.
@@ -93,7 +93,7 @@ export interface FactChecker<
  *
  */
 export interface TechInsightCheckRegistry<CheckType extends TechInsightCheck> {
-  register(check: CheckType): Promise<void>;
+  register(check: CheckType): Promise<CheckType>;
   get(checkId: string): Promise<CheckType>;
   getAll(checks: string[]): Promise<CheckType[]>;
   list(): Promise<CheckType[]>;
@@ -135,7 +135,21 @@ export interface TechInsightCheck {
    */
   id: string;
 
+  /**
+   * Type identifier for the check.
+   * Can be used to determine storage options, logical routing to correct FactChecker implementation
+   * or to help frontend render correct component types based on this
+   */
+  type: string;
+
+  /**
+   * Human readable name of the check, may be displayed in the UI
+   */
   name: string;
+
+  /**
+   * Human readable description of the check, may be displayed in the UI
+   */
   description: string;
 
   /**

plugins/tech-insights-common/src/responses.ts

@@ -26,6 +26,12 @@ export interface CheckResponse {
    * Identifier of the Check
    */
   id: string;
+  /**
+   * Type identifier for the check.
+   * Can be used to determine storage options, logical routing to correct FactChecker implementation
+   * or to help frontend render correct component types based on this
+   */
+  type: string;
   /**
    * Human readable name of the Check
    */