Add validation for port mapping on deploy page (#612)

src/app/frontend/deploy/deploylabel_controller.js

@@ -33,7 +33,6 @@ export default class DeployLabelController {
   /**
    * Calls checks on label:
    *  - adds label if last empty label has been filled
-   *  - removes label if some label in the middle has key and value empty
    *  - checks for duplicated key and sets validity of element
    * @param {!angular.FormController|undefined} labelForm
    * @export
@@ -96,7 +95,7 @@ export default class DeployLabelController {
   validateKey_(labelForm) {
     if (angular.isDefined(labelForm)) {
       /** @type {!angular.NgModelController} */
-      let elem = labelForm.key;
+      let elem = labelForm['key'];
 
       /** @type {boolean} */
       let isUnique = !this.isKeyDuplicated_();

src/app/frontend/deploy/portmappings.html

@@ -16,25 +16,28 @@ limitations under the License.
 
 <div ng-repeat="portMapping in ctrl.portMappings">
   <ng-form name="portMappingForm" layout="row">
-    <md-input-container flex="auto" class="kd-deploy-input-row">
+    <md-input-container flex="50" class="kd-deploy-input-row">
       <label>Port</label>
-      <input ng-model="portMapping.port" ng-change="ctrl.addProtocolIfNeeed()"
+      <input ng-model="portMapping.port" ng-change="ctrl.checkPortMapping(portMappingForm, $index)"
              type="number" min="1" max="65535" name="port">
       <ng-messages for="portMappingForm.port.$error" role="alert" multiple>
         <ng-message when="number">Port must be an integer.</ng-message>
         <ng-message when="min">Port must greater than 0.</ng-message>
         <ng-message when="max">Port must less than 65536.</ng-message>
+        <ng-message when="empty">Port can't be empty when target port is specified.</ng-message>
       </ng-messages>
     </md-input-container>
     <div flex="5"></div>
-    <md-input-container flex="auto" class="kd-deploy-input-row">
+    <md-input-container flex="50" class="kd-deploy-input-row">
       <label>Target port</label>
-      <input ng-model="portMapping.targetPort" ng-change="ctrl.addProtocolIfNeeed()"
+      <input ng-model="portMapping.targetPort"
+             ng-change="ctrl.checkPortMapping(portMappingForm, $index)"
              type="number" min="1" max="65535" name="targetPort">
       <ng-messages for="portMappingForm.targetPort.$error" role="alert" multiple>
         <ng-message when="number">Target port must be an integer.</ng-message>
         <ng-message when="min">Target port must greater than 0.</ng-message>
         <ng-message when="max">Target port must less than 65536.</ng-message>
+        <ng-message when="empty">Target port can't be empty when port is specified.</ng-message>
       </ng-messages>
     </md-input-container>
     <div flex="5"></div>

src/app/frontend/deploy/portmappings_controller.js

@@ -39,6 +39,19 @@ export default class PortMappingsController {
     this.isExternal;
   }
 
+  /**
+   * Call checks on port mapping:
+   *  - adds new port mapping when last empty port mapping has been filled
+   *  - validates port mapping
+   * @param {!angular.FormController|undefined} portMappingForm
+   * @param {number} portMappingIndex
+   * @export
+   */
+  checkPortMapping(portMappingForm, portMappingIndex) {
+    this.addProtocolIfNeeed_();
+    this.validatePortMapping_(portMappingForm, portMappingIndex);
+  }
+
   /**
    * @param {string} defaultProtocol
    * @return {!backendApi.PortMapping}
@@ -51,13 +64,40 @@ export default class PortMappingsController {
   /**
    * @export
    */
-  addProtocolIfNeeed() {
+  addProtocolIfNeeed_() {
     let lastPortMapping = this.portMappings[this.portMappings.length - 1];
     if (this.isPortMappingFilled_(lastPortMapping)) {
       this.portMappings.push(this.newEmptyPortMapping_(this.protocols[0]));
     }
   }
 
+  /**
+   * Validates port mapping. In case when only one port is specified it is considered as invalid.
+   * @param {!angular.FormController|undefined} portMappingForm
+   * @param {number} portIndex
+   * @private
+   */
+  validatePortMapping_(portMappingForm, portIndex) {
+    if (angular.isDefined(portMappingForm)) {
+      /** @type {!backendApi.PortMapping} */
+      let portMapping = this.portMappings[portIndex];
+
+      /** @type {!angular.NgModelController} */
+      let portElem = portMappingForm['port'];
+      /** @type {!angular.NgModelController} */
+      let targetPortElem = portMappingForm['targetPort'];
+
+      /** @type {boolean} */
+      let isValidPort = this.isPortMappingFilledOrEmpty_(portMapping) || !!portMapping.port;
+      /** @type {boolean} */
+      let isValidTargetPort =
+          this.isPortMappingFilledOrEmpty_(portMapping) || !!portMapping.targetPort;
+
+      portElem.$setValidity('empty', isValidPort);
+      targetPortElem.$setValidity('empty', isValidTargetPort);
+    }
+  }
+
   /**
    * @param {number} index
    * @return {boolean}
@@ -78,4 +118,12 @@ export default class PortMappingsController {
    * @private
    */
   isPortMappingFilled_(portMapping) { return !!portMapping.port && !!portMapping.targetPort; }
+
+  /**
+   * Returns true when the given port mapping is filled or empty (both ports), false otherwise.
+   * @param {!backendApi.PortMapping} portMapping
+   * @return {boolean}
+   * @private
+   */
+  isPortMappingFilledOrEmpty_(portMapping) { return !portMapping.port === !portMapping.targetPort; }
 }

src/test/frontend/deploy/portmappings_controller_test.js

@@ -18,12 +18,21 @@ import PortMappingsController from 'deploy/portmappings_controller';
 describe('PortMappingsController controller', () => {
   /** @type {!PortMappingsController} */
   let ctrl;
+  let portMappingForm;
 
   beforeEach(() => {
     angular.mock.module(deployModule.name);
 
-    angular.mock.inject(($controller) => {
+    angular.mock.inject(($controller, $rootScope, $compile) => {
       ctrl = $controller(PortMappingsController, undefined, {protocols: ['FOO', 'BAR']});
+      let scope = $rootScope.$new();
+      let template = angular.element(`<ng-form name="portMappingForm">
+            <input name="port" ng-model="port">
+            <input name="targetPort" ng-model="targetPort">
+           </ng-form>`);
+
+      $compile(template)(scope);
+      portMappingForm = scope.portMappingForm;
     });
   });
 
@@ -34,13 +43,13 @@ describe('PortMappingsController controller', () => {
   it('should add new mappings when needed', () => {
     expect(ctrl.portMappings.length).toBe(1);
 
-    ctrl.addProtocolIfNeeed();
+    ctrl.checkPortMapping(undefined, 0);
     expect(ctrl.portMappings.length).toBe(1);
 
     ctrl.portMappings[0].port = 80;
     ctrl.portMappings[0].targetPort = 8080;
 
-    ctrl.addProtocolIfNeeed();
+    ctrl.checkPortMapping(undefined, 0);
     expect(ctrl.portMappings.length).toBe(2);
   });
 
@@ -48,17 +57,40 @@ describe('PortMappingsController controller', () => {
     expect(ctrl.isRemovable(0)).toBe(false);
     ctrl.portMappings[0].port = 80;
     ctrl.portMappings[0].targetPort = 8080;
-    ctrl.addProtocolIfNeeed();
+    ctrl.checkPortMapping(undefined, 0);
     expect(ctrl.isRemovable(0)).toBe(true);
   });
 
   it('should remove port mappings', () => {
     ctrl.portMappings[0].port = 80;
     ctrl.portMappings[0].targetPort = 8080;
-    ctrl.addProtocolIfNeeed();
+    ctrl.checkPortMapping(undefined, 0);
     expect(ctrl.portMappings.length).toBe(2);
     ctrl.remove(0);
     expect(ctrl.portMappings.length).toBe(1);
     expect(ctrl.portMappings[0].port).toBeNull();
   });
+
+  it('should validate port mapping', () => {
+    let testData = [
+      ['', '', true, true],
+      ['', 80, false, true],
+      [80, '', true, false],
+      [80, 80, true, true],
+    ];
+
+    testData.forEach((testData) => {
+      // given
+      let [port, targetPort, portValidity, targetPortValidity] = testData;
+      ctrl.portMappings[0].port = port;
+      ctrl.portMappings[0].targetPort = targetPort;
+
+      // when
+      ctrl.checkPortMapping(portMappingForm, 0);
+
+      // then
+      expect(portMappingForm.port.$valid).toEqual(portValidity);
+      expect(portMappingForm.targetPort.$valid).toEqual(targetPortValidity);
+    });
+  });
 });