refactor(ngModel): remove $$invalidModelValue and refactor methods

- define `ngModelGet` and `ngModelSet` to already use
  the getter/setter semantics, so the rest of the code does
  not need to care about it.
- remove `ctrl.$$invalidModelValue` to simplify the internal logic

Author: tbosch

src/ng/directive/input.js

@@ -1651,15 +1651,33 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
   this.$name = $attr.name;
 
 
-  var ngModelGet = $parse($attr.ngModel),
-      ngModelSet = ngModelGet.assign,
+  var parsedNgModel = $parse($attr.ngModel),
       pendingDebounce = null,
       ctrl = this;
 
+  var ngModelGet = function ngModelGet() {
+    var modelValue = parsedNgModel($scope);
+    if (ctrl.$options && ctrl.$options.getterSetter && isFunction(modelValue)) {
+      modelValue = modelValue();
+    }
+    return modelValue;
+  };
+
+  var ngModelSet = function ngModelSet(newValue) {
+    var getterSetter;
+    if (ctrl.$options && ctrl.$options.getterSetter &&
+        isFunction(getterSetter = parsedNgModel($scope))) {
+
+      getterSetter(ctrl.$modelValue);
+    } else {
+      parsedNgModel.assign($scope, ctrl.$modelValue);
+    }
+  };
+
   this.$$setOptions = function(options) {
     ctrl.$options = options;
 
-    if (!ngModelSet && (!options || !options.getterSetter)) {
+    if (!parsedNgModel.assign && (!options || !options.getterSetter)) {
       throw $ngModelMinErr('nonassign', "Expression '{0}' is non-assignable. Element: {1}",
           $attr.ngModel, startingTag($element));
     }
@@ -1875,26 +1893,17 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
    * Runs each of the registered validators (first synchronous validators and then asynchronous validators).
    */
   this.$validate = function() {
-    // ignore $validate before model initialized
-    if (ctrl.$modelValue !== ctrl.$modelValue) {
+    // ignore $validate before model is initialized
+    if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
       return;
     }
-
-    var prev = ctrl.$modelValue;
-    ctrl.$$runValidators(undefined, ctrl.$$invalidModelValue || ctrl.$modelValue, ctrl.$viewValue, function() {
-      if (prev !== ctrl.$modelValue) {
-        ctrl.$$writeModelToScope();
-      }
-    });
+    this.$$parseAndValidate();
   };
 
   this.$$runValidators = function(parseValid, modelValue, viewValue, doneCallback) {
     currentValidationRunId++;
     var localValidationRunId = currentValidationRunId;
 
-    // We can update the $$invalidModelValue immediately as we don't have to wait for validators!
-    ctrl.$$invalidModelValue = modelValue;
-
     // check parser error
     if (!processParseErrors(parseValid)) {
       return;
@@ -1971,8 +1980,6 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
 
     function validationDone() {
       if (localValidationRunId === currentValidationRunId) {
-        // set the validated model value
-        ctrl.$modelValue = ctrl.$valid ? modelValue : undefined;
 
         doneCallback();
       }
@@ -2011,31 +2018,31 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
       $animate.addClass($element, DIRTY_CLASS);
       parentForm.$setDirty();
     }
+    this.$$parseAndValidate();
+  };
 
-    var parserValid = true, modelValue = viewValue;
+  this.$$parseAndValidate = function() {
+    var parserValid = true,
+        viewValue = ctrl.$$lastCommittedViewValue,
+        modelValue = viewValue;
     for(var i = 0; i < ctrl.$parsers.length; i++) {
       modelValue = ctrl.$parsers[i](modelValue);
       if (isUndefined(modelValue)) {
         parserValid = false;
         break;
       }
     }
-
+    var prevModelValue = ctrl.$modelValue;
     ctrl.$$runValidators(parserValid, modelValue, viewValue, function() {
-      ctrl.$$writeModelToScope();
+      ctrl.$modelValue = ctrl.$valid ? modelValue : undefined;
+      if (ctrl.$modelValue !== prevModelValue) {
+        ctrl.$$writeModelToScope();
+      }
     });
   };
 
   this.$$writeModelToScope = function() {
-    var getterSetter;
-
-    if (ctrl.$options && ctrl.$options.getterSetter &&
-        isFunction(getterSetter = ngModelGet($scope))) {
-
-      getterSetter(ctrl.$modelValue);
-    } else {
-      ngModelSet($scope, ctrl.$modelValue);
-    }
+    ngModelSet(ctrl.$modelValue);
     forEach(ctrl.$viewChangeListeners, function(listener) {
       try {
         listener();
@@ -2123,17 +2130,20 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
   };
 
   // model -> value
+  // Note: we cannot use a normal scope.$watch as we want to detect the following:
+  // 1. scope value is 'a'
+  // 2. user enters 'b'
+  // 3. ng-change kicks in and reverts scope value to 'a'
+  //    -> scope value did not change since the last digest as
+  //       ng-change executes in apply phase
+  // 4. view should be changed back to 'a'
   $scope.$watch(function ngModelWatch() {
-    var modelValue = ngModelGet($scope);
-
-    if (ctrl.$options && ctrl.$options.getterSetter && isFunction(modelValue)) {
-      modelValue = modelValue();
-    }
+    var modelValue = ngModelGet();
 
     // if scope model value and ngModel value are out of sync
     // TODO(perf): why not move this to the action fn?
-    if (ctrl.$modelValue !== modelValue &&
-        (isUndefined(ctrl.$$invalidModelValue) || ctrl.$$invalidModelValue != modelValue)) {
+    if (modelValue !== ctrl.$modelValue) {
+      ctrl.$modelValue = modelValue;
 
       var formatters = ctrl.$formatters,
           idx = formatters.length;

test/ng/directive/inputSpec.js

@@ -214,6 +214,33 @@ describe('NgModelController', function() {
       expect(ctrl.$modelValue).toBeUndefined();
     });
 
+    it('should not reset the view when the view is invalid', function() {
+      // this test fails when the view changes the model and
+      // then the model listener in ngModel picks up the change and
+      // tries to update the view again.
+
+      // add a validator that will make any input invalid
+      ctrl.$parsers.push(function() {return undefined;});
+      spyOn(ctrl, '$render');
+
+      // first digest
+      ctrl.$setViewValue('bbbb');
+      expect(ctrl.$modelValue).toBeUndefined();
+      expect(ctrl.$viewValue).toBe('bbbb');
+      expect(ctrl.$render).not.toHaveBeenCalled();
+      expect(scope.value).toBeUndefined();
+
+      // further digests
+      scope.$apply('value = "aaa"');
+      expect(ctrl.$viewValue).toBe('aaa');
+      ctrl.$render.reset();
+
+      ctrl.$setViewValue('cccc');
+      expect(ctrl.$modelValue).toBeUndefined();
+      expect(ctrl.$viewValue).toBe('cccc');
+      expect(ctrl.$render).not.toHaveBeenCalled();
+      expect(scope.value).toBeUndefined();
+    });
 
     it('should call parentForm.$setDirty only when pristine', function() {
       ctrl.$setViewValue('');
@@ -385,18 +412,18 @@ describe('NgModelController', function() {
   describe('validations pipeline', function() {
 
     it('should perform validations when $validate() is called', function() {
-      ctrl.$validators.uppercase = function(value) {
-        return (/^[A-Z]+$/).test(value);
+      scope.$apply('value = ""');
+
+      var validatorResult = false;
+      ctrl.$validators.someValidator = function(value) {
+        return validatorResult;
       };
 
-      ctrl.$modelValue = 'test';
-      ctrl.$$invalidModelValue = undefined;
       ctrl.$validate();
 
       expect(ctrl.$valid).toBe(false);
 
-      ctrl.$modelValue = 'TEST';
-      ctrl.$$invalidModelValue = undefined;
+      validatorResult = true;
       ctrl.$validate();
 
       expect(ctrl.$valid).toBe(true);
@@ -3936,6 +3963,13 @@ describe('input', function() {
       browserTrigger(inputElm, 'click');
       expect(scope.changeFn).toHaveBeenCalledOnce();
     });
+
+    it('should be able to change the model and via that also update the view', function() {
+      compileInput('<input type="text" ng-model="value" ng-change="value=\'b\'" />');
+
+      changeInputValueTo('a');
+      expect(inputElm.val()).toBe('b');
+    });
   });