You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Tom Beerbower <tb...@hortonworks.com> on 2014/11/12 15:11:47 UTC

Review Request 27913: Views: support validation

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/
-----------------------------------------------------------

Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.


Bugs: AMBARI-8295
    https://issues.apache.org/jira/browse/AMBARI-8295


Repository: ambari


Description
-------

Proposal:
The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.

{code}
public interface Validator {
  /**
   * Validate the given view instance definition.  Return {@code null} to indicate that
   * no validation was performed.
   *
   * @param definition  the view instance definition
   * @param mode        the validation mode
   *
   * @return the instance validation result; may be {@code null}
   */
  public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);

  /**
   * Validate a property of the given view instance definition.  Return {@code null} to indicate that
   * no validation was performed.
   *
   * @param property    the property name
   * @param definition  the view instance definition
   * @param mode        the validation mode
   *
   * @return the instance validation result; may be {@code null}
   */
  public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);

  /**
   * The context in which a view instance validation check is performed.
   */
  public enum ValidationContext {
    PRE_CREATE,  // validation prior to creation
    PRE_UPDATE,  // validation prior to update
    EXISTING     // validation of an existing view instance
  }
}
{code}

The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 

So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).

The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
{code}
public interface ValidationResult {

  /**
   * Determine whether or not the result is valid.
   *
   * @return true if the result is valid
   */
  public boolean isValid();

  /**
   * Get the detail of the validation result.
   *
   * @return the validation result detail
   */
  public String getDetail();

  /**
   * Successful validation result.
   */
  public static final ValidationResult SUCCESS = new ValidationResult() {
    @Override
    public boolean isValid() {
      return true;
    }

    @Override
    public String getDetail() {
      return "OK";
    }
  };
}
{code}


If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...

{code}
{
  "status": 400,
  "message": "{"propertyResults": {
    "cities": {
        "valid": true,
        "detail": "OK"
    },
    "units": {
        "valid": false,
        "detail": "Units must be either metric or imperial."
    }
}, "valid": false, "detail": "The instance has invalid properties."}"
}
{code}


When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...

{code}
{
  "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
  "ViewInstanceInfo" : {
    "instance_name" : "EUROPE",
    ...
    "validation_result" : {
      "valid" : true,
      "detail" : "OK"
    },
    "version" : "1.0.0",
    "view_name" : "WEATHER",
    ...
    "properties" : {
      "cities" : "London, UK;Paris;Munich",
      "units" : "imperial"
    },
    "property_validation_results" : {
      "cities" : {
        "valid" : true,
        "detail" : "OK"
      },
      "units" : {
        "valid" : true,
        "detail" : "OK"
      }
    }
  },
...
{code}

Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
  ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
  ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
  ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
  ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
  ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
  ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
  ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
  ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
  ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
  ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
  ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
  ambari-views/src/main/resources/view.xsd b95e4ec 

Diff: https://reviews.apache.org/r/27913/diff/


Testing
-------

Manual tests.

New unit tests added.  All existing tests pass ...

Results :

Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14


[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 24:20 min
[INFO] Finished at: 2014-11-12T07:51:20-05:00
[INFO] Final Memory: 41M/351M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower


Re: Review Request 27913: Views: support validation

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/#review60999
-----------------------------------------------------------

Ship it!


Ship It!

- Jonathan Hurley


On Nov. 12, 2014, 11:38 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27913/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2014, 11:38 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-8295
>     https://issues.apache.org/jira/browse/AMBARI-8295
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Proposal:
> The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.
> 
> {code}
> public interface Validator {
>   /**
>    * Validate the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * Validate a property of the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param property    the property name
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * The context in which a view instance validation check is performed.
>    */
>   public enum ValidationContext {
>     PRE_CREATE,  // validation prior to creation
>     PRE_UPDATE,  // validation prior to update
>     EXISTING     // validation of an existing view instance
>   }
> }
> {code}
> 
> The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 
> 
> So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).
> 
> The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
> {code}
> public interface ValidationResult {
> 
>   /**
>    * Determine whether or not the result is valid.
>    *
>    * @return true if the result is valid
>    */
>   public boolean isValid();
> 
>   /**
>    * Get the detail of the validation result.
>    *
>    * @return the validation result detail
>    */
>   public String getDetail();
> 
>   /**
>    * Successful validation result.
>    */
>   public static final ValidationResult SUCCESS = new ValidationResult() {
>     @Override
>     public boolean isValid() {
>       return true;
>     }
> 
>     @Override
>     public String getDetail() {
>       return "OK";
>     }
>   };
> }
> {code}
> 
> 
> If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...
> 
> {code}
> {
>   "status": 400,
>   "message": "{"propertyResults": {
>     "cities": {
>         "valid": true,
>         "detail": "OK"
>     },
>     "units": {
>         "valid": false,
>         "detail": "Units must be either metric or imperial."
>     }
> }, "valid": false, "detail": "The instance has invalid properties."}"
> }
> {code}
> 
> 
> When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...
> 
> {code}
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
>   "ViewInstanceInfo" : {
>     "instance_name" : "EUROPE",
>     ...
>     "validation_result" : {
>       "valid" : true,
>       "detail" : "OK"
>     },
>     "version" : "1.0.0",
>     "view_name" : "WEATHER",
>     ...
>     "properties" : {
>       "cities" : "London, UK;Paris;Munich",
>       "units" : "imperial"
>     },
>     "property_validation_results" : {
>       "cities" : {
>         "valid" : true,
>         "detail" : "OK"
>       },
>       "units" : {
>         "valid" : true,
>         "detail" : "OK"
>       }
>     }
>   },
> ...
> {code}
> 
> Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
>   ambari-views/src/main/resources/view.xsd b95e4ec 
> 
> Diff: https://reviews.apache.org/r/27913/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit tests added.  All existing tests pass ...
> 
> Results :
> 
> Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:20 min
> [INFO] Finished at: 2014-11-12T07:51:20-05:00
> [INFO] Final Memory: 41M/351M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 27913: Views: support validation

Posted by Tom Beerbower <tb...@hortonworks.com>.

> On Nov. 12, 2014, 7:46 p.m., Alejandro Fernandez wrote:
> > Nice, this should be useful for the Jobs View to validate that the URLs and ports are correctly formatted and reachable. FYI, https://mathiasbynens.be/demo/url-regex

Thanks for reviewing!


- Tom


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/#review61047
-----------------------------------------------------------


On Nov. 12, 2014, 4:38 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27913/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2014, 4:38 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-8295
>     https://issues.apache.org/jira/browse/AMBARI-8295
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Proposal:
> The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.
> 
> {code}
> public interface Validator {
>   /**
>    * Validate the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * Validate a property of the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param property    the property name
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * The context in which a view instance validation check is performed.
>    */
>   public enum ValidationContext {
>     PRE_CREATE,  // validation prior to creation
>     PRE_UPDATE,  // validation prior to update
>     EXISTING     // validation of an existing view instance
>   }
> }
> {code}
> 
> The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 
> 
> So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).
> 
> The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
> {code}
> public interface ValidationResult {
> 
>   /**
>    * Determine whether or not the result is valid.
>    *
>    * @return true if the result is valid
>    */
>   public boolean isValid();
> 
>   /**
>    * Get the detail of the validation result.
>    *
>    * @return the validation result detail
>    */
>   public String getDetail();
> 
>   /**
>    * Successful validation result.
>    */
>   public static final ValidationResult SUCCESS = new ValidationResult() {
>     @Override
>     public boolean isValid() {
>       return true;
>     }
> 
>     @Override
>     public String getDetail() {
>       return "OK";
>     }
>   };
> }
> {code}
> 
> 
> If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...
> 
> {code}
> {
>   "status": 400,
>   "message": "{"propertyResults": {
>     "cities": {
>         "valid": true,
>         "detail": "OK"
>     },
>     "units": {
>         "valid": false,
>         "detail": "Units must be either metric or imperial."
>     }
> }, "valid": false, "detail": "The instance has invalid properties."}"
> }
> {code}
> 
> 
> When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...
> 
> {code}
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
>   "ViewInstanceInfo" : {
>     "instance_name" : "EUROPE",
>     ...
>     "validation_result" : {
>       "valid" : true,
>       "detail" : "OK"
>     },
>     "version" : "1.0.0",
>     "view_name" : "WEATHER",
>     ...
>     "properties" : {
>       "cities" : "London, UK;Paris;Munich",
>       "units" : "imperial"
>     },
>     "property_validation_results" : {
>       "cities" : {
>         "valid" : true,
>         "detail" : "OK"
>       },
>       "units" : {
>         "valid" : true,
>         "detail" : "OK"
>       }
>     }
>   },
> ...
> {code}
> 
> Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
>   ambari-views/src/main/resources/view.xsd b95e4ec 
> 
> Diff: https://reviews.apache.org/r/27913/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit tests added.  All existing tests pass ...
> 
> Results :
> 
> Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:20 min
> [INFO] Finished at: 2014-11-12T07:51:20-05:00
> [INFO] Final Memory: 41M/351M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 27913: Views: support validation

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/#review61047
-----------------------------------------------------------

Ship it!


Nice, this should be useful for the Jobs View to validate that the URLs and ports are correctly formatted and reachable. FYI, https://mathiasbynens.be/demo/url-regex

- Alejandro Fernandez


On Nov. 12, 2014, 4:38 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27913/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2014, 4:38 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-8295
>     https://issues.apache.org/jira/browse/AMBARI-8295
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Proposal:
> The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.
> 
> {code}
> public interface Validator {
>   /**
>    * Validate the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * Validate a property of the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param property    the property name
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * The context in which a view instance validation check is performed.
>    */
>   public enum ValidationContext {
>     PRE_CREATE,  // validation prior to creation
>     PRE_UPDATE,  // validation prior to update
>     EXISTING     // validation of an existing view instance
>   }
> }
> {code}
> 
> The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 
> 
> So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).
> 
> The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
> {code}
> public interface ValidationResult {
> 
>   /**
>    * Determine whether or not the result is valid.
>    *
>    * @return true if the result is valid
>    */
>   public boolean isValid();
> 
>   /**
>    * Get the detail of the validation result.
>    *
>    * @return the validation result detail
>    */
>   public String getDetail();
> 
>   /**
>    * Successful validation result.
>    */
>   public static final ValidationResult SUCCESS = new ValidationResult() {
>     @Override
>     public boolean isValid() {
>       return true;
>     }
> 
>     @Override
>     public String getDetail() {
>       return "OK";
>     }
>   };
> }
> {code}
> 
> 
> If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...
> 
> {code}
> {
>   "status": 400,
>   "message": "{"propertyResults": {
>     "cities": {
>         "valid": true,
>         "detail": "OK"
>     },
>     "units": {
>         "valid": false,
>         "detail": "Units must be either metric or imperial."
>     }
> }, "valid": false, "detail": "The instance has invalid properties."}"
> }
> {code}
> 
> 
> When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...
> 
> {code}
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
>   "ViewInstanceInfo" : {
>     "instance_name" : "EUROPE",
>     ...
>     "validation_result" : {
>       "valid" : true,
>       "detail" : "OK"
>     },
>     "version" : "1.0.0",
>     "view_name" : "WEATHER",
>     ...
>     "properties" : {
>       "cities" : "London, UK;Paris;Munich",
>       "units" : "imperial"
>     },
>     "property_validation_results" : {
>       "cities" : {
>         "valid" : true,
>         "detail" : "OK"
>       },
>       "units" : {
>         "valid" : true,
>         "detail" : "OK"
>       }
>     }
>   },
> ...
> {code}
> 
> Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
>   ambari-views/src/main/resources/view.xsd b95e4ec 
> 
> Diff: https://reviews.apache.org/r/27913/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit tests added.  All existing tests pass ...
> 
> Results :
> 
> Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:20 min
> [INFO] Finished at: 2014-11-12T07:51:20-05:00
> [INFO] Final Memory: 41M/351M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 27913: Views: support validation

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/
-----------------------------------------------------------

(Updated Nov. 12, 2014, 4:38 p.m.)


Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.


Bugs: AMBARI-8295
    https://issues.apache.org/jira/browse/AMBARI-8295


Repository: ambari


Description
-------

Proposal:
The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.

{code}
public interface Validator {
  /**
   * Validate the given view instance definition.  Return {@code null} to indicate that
   * no validation was performed.
   *
   * @param definition  the view instance definition
   * @param mode        the validation mode
   *
   * @return the instance validation result; may be {@code null}
   */
  public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);

  /**
   * Validate a property of the given view instance definition.  Return {@code null} to indicate that
   * no validation was performed.
   *
   * @param property    the property name
   * @param definition  the view instance definition
   * @param mode        the validation mode
   *
   * @return the instance validation result; may be {@code null}
   */
  public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);

  /**
   * The context in which a view instance validation check is performed.
   */
  public enum ValidationContext {
    PRE_CREATE,  // validation prior to creation
    PRE_UPDATE,  // validation prior to update
    EXISTING     // validation of an existing view instance
  }
}
{code}

The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 

So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).

The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
{code}
public interface ValidationResult {

  /**
   * Determine whether or not the result is valid.
   *
   * @return true if the result is valid
   */
  public boolean isValid();

  /**
   * Get the detail of the validation result.
   *
   * @return the validation result detail
   */
  public String getDetail();

  /**
   * Successful validation result.
   */
  public static final ValidationResult SUCCESS = new ValidationResult() {
    @Override
    public boolean isValid() {
      return true;
    }

    @Override
    public String getDetail() {
      return "OK";
    }
  };
}
{code}


If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...

{code}
{
  "status": 400,
  "message": "{"propertyResults": {
    "cities": {
        "valid": true,
        "detail": "OK"
    },
    "units": {
        "valid": false,
        "detail": "Units must be either metric or imperial."
    }
}, "valid": false, "detail": "The instance has invalid properties."}"
}
{code}


When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...

{code}
{
  "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
  "ViewInstanceInfo" : {
    "instance_name" : "EUROPE",
    ...
    "validation_result" : {
      "valid" : true,
      "detail" : "OK"
    },
    "version" : "1.0.0",
    "view_name" : "WEATHER",
    ...
    "properties" : {
      "cities" : "London, UK;Paris;Munich",
      "units" : "imperial"
    },
    "property_validation_results" : {
      "cities" : {
        "valid" : true,
        "detail" : "OK"
      },
      "units" : {
        "valid" : true,
        "detail" : "OK"
      }
    }
  },
...
{code}

Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
  ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
  ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
  ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
  ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
  ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
  ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
  ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
  ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
  ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
  ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
  ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
  ambari-views/src/main/resources/view.xsd b95e4ec 

Diff: https://reviews.apache.org/r/27913/diff/


Testing
-------

Manual tests.

New unit tests added.  All existing tests pass ...

Results :

Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14


[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 24:20 min
[INFO] Finished at: 2014-11-12T07:51:20-05:00
[INFO] Final Memory: 41M/351M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower


Re: Review Request 27913: Views: support validation

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/
-----------------------------------------------------------

(Updated Nov. 12, 2014, 4:28 p.m.)


Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.


Changes
-------

Updated patch.


Bugs: AMBARI-8295
    https://issues.apache.org/jira/browse/AMBARI-8295


Repository: ambari


Description
-------

Proposal:
The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.

{code}
public interface Validator {
  /**
   * Validate the given view instance definition.  Return {@code null} to indicate that
   * no validation was performed.
   *
   * @param definition  the view instance definition
   * @param mode        the validation mode
   *
   * @return the instance validation result; may be {@code null}
   */
  public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);

  /**
   * Validate a property of the given view instance definition.  Return {@code null} to indicate that
   * no validation was performed.
   *
   * @param property    the property name
   * @param definition  the view instance definition
   * @param mode        the validation mode
   *
   * @return the instance validation result; may be {@code null}
   */
  public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);

  /**
   * The context in which a view instance validation check is performed.
   */
  public enum ValidationContext {
    PRE_CREATE,  // validation prior to creation
    PRE_UPDATE,  // validation prior to update
    EXISTING     // validation of an existing view instance
  }
}
{code}

The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 

So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).

The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
{code}
public interface ValidationResult {

  /**
   * Determine whether or not the result is valid.
   *
   * @return true if the result is valid
   */
  public boolean isValid();

  /**
   * Get the detail of the validation result.
   *
   * @return the validation result detail
   */
  public String getDetail();

  /**
   * Successful validation result.
   */
  public static final ValidationResult SUCCESS = new ValidationResult() {
    @Override
    public boolean isValid() {
      return true;
    }

    @Override
    public String getDetail() {
      return "OK";
    }
  };
}
{code}


If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...

{code}
{
  "status": 400,
  "message": "{"propertyResults": {
    "cities": {
        "valid": true,
        "detail": "OK"
    },
    "units": {
        "valid": false,
        "detail": "Units must be either metric or imperial."
    }
}, "valid": false, "detail": "The instance has invalid properties."}"
}
{code}


When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...

{code}
{
  "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
  "ViewInstanceInfo" : {
    "instance_name" : "EUROPE",
    ...
    "validation_result" : {
      "valid" : true,
      "detail" : "OK"
    },
    "version" : "1.0.0",
    "view_name" : "WEATHER",
    ...
    "properties" : {
      "cities" : "London, UK;Paris;Munich",
      "units" : "imperial"
    },
    "property_validation_results" : {
      "cities" : {
        "valid" : true,
        "detail" : "OK"
      },
      "units" : {
        "valid" : true,
        "detail" : "OK"
      }
    }
  },
...
{code}

Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
  ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
  ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
  ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
  ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
  ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
  ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
  ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
  ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
  ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
  ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
  ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
  ambari-views/src/main/resources/view.xsd b95e4ec 

Diff: https://reviews.apache.org/r/27913/diff/


Testing
-------

Manual tests.

New unit tests added.  All existing tests pass ...

Results :

Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14


[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 24:20 min
[INFO] Finished at: 2014-11-12T07:51:20-05:00
[INFO] Final Memory: 41M/351M
[INFO] ------------------------------------------------------------------------


Thanks,

Tom Beerbower


Re: Review Request 27913: Views: support validation

Posted by Tom Beerbower <tb...@hortonworks.com>.

> On Nov. 12, 2014, 3:17 p.m., Jonathan Hurley wrote:
> >

Thanks for reviewing!  Update patch coming soon.


> On Nov. 12, 2014, 3:17 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java, line 268
> > <https://reviews.apache.org/r/27913/diff/1/?file=759580#file759580line268>
> >
> >     Should this be a separate method called isValidationRequired() or something similar? It looks like this is getting the validator and then determining based on whether the validator is null whether or not validation is required.
> >     
> >     It's a nit, but if the validator isn't needed at this point, then a boolean should be enough.

Yes, good catch.  I was initially using the validator here but refactored a bunch of common code.  A boolean is good enough.


> On Nov. 12, 2014, 3:17 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java, line 1056
> > <https://reviews.apache.org/r/27913/diff/1/?file=759583#file759583line1056>
> >
> >     Does this need to be static? It doesn't appear to be accessed statically.

Doesn't need to be static but nothing in the method is tied to an instance.  It's really just a private helper method.


> On Nov. 12, 2014, 3:17 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java, line 1333
> > <https://reviews.apache.org/r/27913/diff/1/?file=759583#file759583line1333>
> >
> >     Potential NPE here? Or is viewDefinition being null at this point not really possible?

Should be okay.  The viewDefinition is new'd just before calling this.  It's private so I don't have to worry about unintended usages.


> On Nov. 12, 2014, 3:17 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java, line 69
> > <https://reviews.apache.org/r/27913/diff/1/?file=759585#file759585line69>
> >
> >     Gson is thread-safe to reuse, but costly to instantiate. I would say make this static and just reuse the Gson instance that's built once.

Good point.  Thanks!


- Tom


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/#review60989
-----------------------------------------------------------


On Nov. 12, 2014, 2:11 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27913/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2014, 2:11 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-8295
>     https://issues.apache.org/jira/browse/AMBARI-8295
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Proposal:
> The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.
> 
> {code}
> public interface Validator {
>   /**
>    * Validate the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * Validate a property of the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param property    the property name
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * The context in which a view instance validation check is performed.
>    */
>   public enum ValidationContext {
>     PRE_CREATE,  // validation prior to creation
>     PRE_UPDATE,  // validation prior to update
>     EXISTING     // validation of an existing view instance
>   }
> }
> {code}
> 
> The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 
> 
> So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).
> 
> The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
> {code}
> public interface ValidationResult {
> 
>   /**
>    * Determine whether or not the result is valid.
>    *
>    * @return true if the result is valid
>    */
>   public boolean isValid();
> 
>   /**
>    * Get the detail of the validation result.
>    *
>    * @return the validation result detail
>    */
>   public String getDetail();
> 
>   /**
>    * Successful validation result.
>    */
>   public static final ValidationResult SUCCESS = new ValidationResult() {
>     @Override
>     public boolean isValid() {
>       return true;
>     }
> 
>     @Override
>     public String getDetail() {
>       return "OK";
>     }
>   };
> }
> {code}
> 
> 
> If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...
> 
> {code}
> {
>   "status": 400,
>   "message": "{"propertyResults": {
>     "cities": {
>         "valid": true,
>         "detail": "OK"
>     },
>     "units": {
>         "valid": false,
>         "detail": "Units must be either metric or imperial."
>     }
> }, "valid": false, "detail": "The instance has invalid properties."}"
> }
> {code}
> 
> 
> When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...
> 
> {code}
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
>   "ViewInstanceInfo" : {
>     "instance_name" : "EUROPE",
>     ...
>     "validation_result" : {
>       "valid" : true,
>       "detail" : "OK"
>     },
>     "version" : "1.0.0",
>     "view_name" : "WEATHER",
>     ...
>     "properties" : {
>       "cities" : "London, UK;Paris;Munich",
>       "units" : "imperial"
>     },
>     "property_validation_results" : {
>       "cities" : {
>         "valid" : true,
>         "detail" : "OK"
>       },
>       "units" : {
>         "valid" : true,
>         "detail" : "OK"
>       }
>     }
>   },
> ...
> {code}
> 
> Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
>   ambari-views/src/main/resources/view.xsd b95e4ec 
> 
> Diff: https://reviews.apache.org/r/27913/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit tests added.  All existing tests pass ...
> 
> Results :
> 
> Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:20 min
> [INFO] Finished at: 2014-11-12T07:51:20-05:00
> [INFO] Final Memory: 41M/351M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Re: Review Request 27913: Views: support validation

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27913/#review60989
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java
<https://reviews.apache.org/r/27913/#comment102431>

    Should this be a separate method called isValidationRequired() or something similar? It looks like this is getting the validator and then determining based on whether the validator is null whether or not validation is required.
    
    It's a nit, but if the validator isn't needed at this point, then a boolean should be enough.



ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java
<https://reviews.apache.org/r/27913/#comment102432>

    Does this need to be static? It doesn't appear to be accessed statically.



ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java
<https://reviews.apache.org/r/27913/#comment102433>

    Potential NPE here? Or is viewDefinition being null at this point not really possible?



ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java
<https://reviews.apache.org/r/27913/#comment102435>

    Gson is thread-safe to reuse, but costly to instantiate. I would say make this static and just reuse the Gson instance that's built once.


- Jonathan Hurley


On Nov. 12, 2014, 9:11 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27913/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2014, 9:11 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-8295
>     https://issues.apache.org/jira/browse/AMBARI-8295
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Proposal:
> The view framework SPI exposes a new Validator interface that could optionally be implemented by the view developer. The Validator interface allows the view developer to define the validation logic that gets applied when a new view instance is created or updated. The interface specifies a svalidate method that is passed a ViewInstanceDefinition. The given view instance is the instance that is being created or updated. The validator has full access to the instance definition as well as the owning view definition so it can check things like parameters and properties. If the validator successfully validates the given instance it simply returns. If the validation fails then the validator should throw an ValidationException with a message that describes the failure.
> 
> {code}
> public interface Validator {
>   /**
>    * Validate the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateInstance(ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * Validate a property of the given view instance definition.  Return {@code null} to indicate that
>    * no validation was performed.
>    *
>    * @param property    the property name
>    * @param definition  the view instance definition
>    * @param mode        the validation mode
>    *
>    * @return the instance validation result; may be {@code null}
>    */
>   public ValidationResult validateProperty(String property, ViewInstanceDefinition definition, ValidationContext mode);
> 
>   /**
>    * The context in which a view instance validation check is performed.
>    */
>   public enum ValidationContext {
>     PRE_CREATE,  // validation prior to creation
>     PRE_UPDATE,  // validation prior to update
>     EXISTING     // validation of an existing view instance
>   }
> }
> {code}
> 
> The context allows the framework a way to let the validator know at what point in the view instance lifecycle the validation is being performed.  If a validation fails during instance create (PRE_CREATE), the view framework will rollback the instance creation and will fail with an appropriate response (see below).  The same is true for instance update (PRE_UPDATE).  The validator (if specified) will also be called during a GET of view instance resources (EXISTING) and be reported back as properties of the resource in the normal response. 
> 
> So, for example, a view instance may have a 'server' property.  During instance creation (PRE_CREATE), the validator for the view might check the 'server' property for invalid characters and fail the create if found.  After an instance is created and a request to get the instance resource is made (EXISTING), the validator might check to see if the server specified by the 'server' property is actually reachable and fail the verify if not.  In this case the view instance resource is still reachable but the fact that the validation failed will show as a property in the instance resource (see below).
> 
> The result of the validation for the view instance and each of its properties are returned from the validator to the view framework in the validation result classes ...
> {code}
> public interface ValidationResult {
> 
>   /**
>    * Determine whether or not the result is valid.
>    *
>    * @return true if the result is valid
>    */
>   public boolean isValid();
> 
>   /**
>    * Get the detail of the validation result.
>    *
>    * @return the validation result detail
>    */
>   public String getDetail();
> 
>   /**
>    * Successful validation result.
>    */
>   public static final ValidationResult SUCCESS = new ValidationResult() {
>     @Override
>     public boolean isValid() {
>       return true;
>     }
> 
>     @Override
>     public String getDetail() {
>       return "OK";
>     }
>   };
> }
> {code}
> 
> 
> If a POST or PUT for a view instance fails because of a validation check the results will look something like this (note in the example that the validation for one property passed while the other failed) ...
> 
> {code}
> {
>   "status": 400,
>   "message": "{"propertyResults": {
>     "cities": {
>         "valid": true,
>         "detail": "OK"
>     },
>     "units": {
>         "valid": false,
>         "detail": "Units must be either metric or imperial."
>     }
> }, "valid": false, "detail": "The instance has invalid properties."}"
> }
> {code}
> 
> 
> When a view specifies a validator, the instance resource will include the properties 'validation_result' and 'property_validation_results' which will be reevaluated each time the resource is requested...
> 
> {code}
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/views/WEATHER/versions/1.0.0/instances/EUROPE",
>   "ViewInstanceInfo" : {
>     "instance_name" : "EUROPE",
>     ...
>     "validation_result" : {
>       "valid" : true,
>       "detail" : "OK"
>     },
>     "version" : "1.0.0",
>     "view_name" : "WEATHER",
>     ...
>     "properties" : {
>       "cities" : "London, UK;Paris;Munich",
>       "units" : "imperial"
>     },
>     "property_validation_results" : {
>       "cities" : {
>         "valid" : true,
>         "detail" : "OK"
>       },
>       "units" : {
>         "valid" : true,
>         "detail" : "OK"
>       }
>     }
>   },
> ...
> {code}
> 
> Note that in the examples above, the detail messages are provided by the view Validator implementation and are specific to the view (it can be any text that makes sense for that view).
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b4d582a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewInstanceResourceProvider.java 3f62cc3 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewEntity.java d42e1a0 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ViewInstanceEntity.java efa2818 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java a5d11bd 
>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java 3a23ea7 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/InstanceValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/view/validation/ValidationResultImpl.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java fc8acd0 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewEntityTest.java 965cebb 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/ViewInstanceEntityTest.java 08aea6e 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java f6ec403 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/InstanceValidationResultImplTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/view/validation/ValidationResultImplTest.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/ValidationResult.java PRE-CREATION 
>   ambari-views/src/main/java/org/apache/ambari/view/validation/Validator.java PRE-CREATION 
>   ambari-views/src/main/resources/view.xsd b95e4ec 
> 
> Diff: https://reviews.apache.org/r/27913/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit tests added.  All existing tests pass ...
> 
> Results :
> 
> Tests run: 2244, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 24:20 min
> [INFO] Finished at: 2014-11-12T07:51:20-05:00
> [INFO] Final Memory: 41M/351M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>