From cd62a34588ca733faf27049a6a64c79678e49523 Mon Sep 17 00:00:00 2001
From: Jammy Louie <jlouie@pivotal.io>
Date: Wed, 5 Dec 2018 15:46:58 -0500
Subject: [PATCH] fix(provider/cf): make validators more robust (#6047)

- improve error messages for instance size validation
- trim JSON when checking input size

Co-Authored-By: Jammy Louie <jlouie@pivotal.io>
---
 .../validation/instanceSize.validator.ts      | 24 ++++++++++++------
 .../validServiceParameterJson.validator.ts    | 25 +++++++++++++------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/instanceSize.validator.ts b/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/instanceSize.validator.ts
index 0a07be6e1f0..81fe8ec060c 100644
--- a/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/instanceSize.validator.ts
+++ b/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/instanceSize.validator.ts
@@ -35,12 +35,21 @@ export class CfInstanceSizeFieldValidator implements IStageOrTriggerValidator {
 
   protected validationMessage(validationConfig: IInstanceFieldSizeValidationConfig, pipeline: IPipeline): string {
     const fieldLabel: string = this.printableFieldLabel(validationConfig);
-    const min: any = has(validationConfig, 'min') ? get(validationConfig, 'min') : 'NA';
-    const max: any = has(validationConfig, 'max') ? get(validationConfig, 'max') : 'NA';
-    return (
-      validationConfig.message ||
-      `<strong>${fieldLabel}</strong> should be between ${min} and ${max} in ${pipeline.name}`
-    );
+    const hasMin = has(validationConfig, 'min');
+    const min: any = hasMin ? get(validationConfig, 'min') : 'NA';
+    const hasMax = has(validationConfig, 'max');
+    const max: any = hasMax ? get(validationConfig, 'max') : 'NA';
+    let message = ``;
+    if (hasMin) {
+      if (hasMax) {
+        message = `<strong>${fieldLabel}</strong> should be from ${min} to ${max} in ${pipeline.name}.`;
+      } else {
+        message = `<strong>${fieldLabel}</strong> should be at least ${min} in ${pipeline.name}.`;
+      }
+    } else if (hasMax) {
+      message = `<strong>${fieldLabel}</strong> should be no more than ${max} in ${pipeline.name}.`;
+    }
+    return validationConfig.message || message;
   }
 
   protected printableFieldLabel(config: IInstanceFieldSizeValidationConfig): string {
@@ -56,8 +65,7 @@ export class CfInstanceSizeFieldValidator implements IStageOrTriggerValidator {
     const max: number = get(config, 'max');
     const min: number = get(config, 'min');
 
-    const result: any = fieldExists && ((!hasMax || (hasMax && field <= max)) && (!hasMin || (hasMin && field >= min)));
-    return result;
+    return fieldExists && ((!hasMax || (hasMax && field <= max)) && (!hasMin || (hasMin && field >= min)));
   }
 }
 
diff --git a/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/validServiceParameterJson.validator.ts b/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/validServiceParameterJson.validator.ts
index 55d1eaa29a0..227f0cbf99d 100644
--- a/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/validServiceParameterJson.validator.ts
+++ b/app/scripts/modules/cloudfoundry/src/pipeline/config/validation/validServiceParameterJson.validator.ts
@@ -17,31 +17,40 @@ export interface IServiceParameterJsonValidationConfig extends IValidatorConfig
 
 export class ServiceParameterJsonFieldValidator implements IStageOrTriggerValidator {
   public validate(
-    _pipeline: IPipeline,
+    pipeline: IPipeline,
     stage: IStage | ITrigger,
     validationConfig: IServiceParameterJsonValidationConfig,
   ): string {
-    if (!this.fieldIsValid(stage, validationConfig)) {
-      return this.validationMessage(validationConfig);
+    if (!this.passesValidation(stage, validationConfig)) {
+      return this.validationMessage(validationConfig, pipeline);
     }
     return null;
   }
 
-  private validationMessage(validationConfig: IServiceParameterJsonValidationConfig): string {
+  protected passesValidation(
+    stage: IStage | ITrigger,
+    validationConfig: IServiceParameterJsonValidationConfig,
+  ): boolean {
+    return this.fieldIsValid(stage, validationConfig);
+  }
+
+  protected validationMessage(validationConfig: IServiceParameterJsonValidationConfig, pipeline: IPipeline): string {
     const fieldLabel: string = this.printableFieldLabel(validationConfig);
-    return validationConfig.message || `<strong>${fieldLabel}</strong> should be a valid JSON string.`;
+    return (
+      validationConfig.message || `<strong>${fieldLabel}</strong> should be a valid JSON string in ${pipeline.name}`
+    );
   }
 
-  private printableFieldLabel(config: IServiceParameterJsonValidationConfig): string {
+  protected printableFieldLabel(config: IServiceParameterJsonValidationConfig): string {
     const fieldLabel: string = config.fieldLabel || config.fieldName;
     return upperFirst(fieldLabel);
   }
 
-  private fieldIsValid(stage: IStage | ITrigger, config: IServiceParameterJsonValidationConfig): boolean {
+  protected fieldIsValid(stage: IStage | ITrigger, config: IServiceParameterJsonValidationConfig): boolean {
     const fieldExists = has(stage, config.fieldName);
     const field: any = get(stage, config.fieldName);
 
-    if (!fieldExists || !field) {
+    if (!fieldExists || !field.trim()) {
       return true;
     }