[jira] [Commented] (SLING-4027) Improvement of the validation API
[ https://issues.apache.org/jira/browse/SLING-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14581729#comment-14581729 ] Konrad Windszus commented on SLING-4027: f) was solved in SLING-4777. Since all the points mentioned here have been addressed in related issues I consider this fixed. Improvement of the validation API - Key: SLING-4027 URL: https://issues.apache.org/jira/browse/SLING-4027 Project: Sling Issue Type: Improvement Components: Validation Reporter: Carsten Ziegeler Assignee: Konrad Windszus Fix For: Validation 1.0.0 Some comments / thoughts about the validation api: a) Why is there a validator lookup service? I don't think we need this in the API - it's a simple OSGI service lookup. b) A Validator can only validate a single value - what if a property is an array and the validation needs to validate based on all supplied values? Same goes with dependencies between two properties? c) The Validator interface returns null on success and a String (message) if validation fails. But it can also throw an exception if e.g. the provided value is null. I think a null value should be treated the same as a wrong value. Throwing the exception if some configuration like the regexp for the regexp validator is missing, is fine. but all errors of validating a value should be treated the same. d) NonExistingTypeException I don't think we need this - IllegalArgumentException is fine to throw from the type enumeration e) Maybe we can also remove the SlingValidationException - it is only thrown (see c) if a validator does not get its required configuration - which can be seen as an IllegalStateException f) It would be nice to have a ValidationModelProvider interface - we will then have the current way of defining models as the default implememtation. But can allow other means of defining the validation model -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (SLING-4027) Improvement of the validation API
[
https://issues.apache.org/jira/browse/SLING-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14219170#comment-14219170
]
Radu Cotescu commented on SLING-4027:
-
[~cziegeler], the {{ValidatorLookupService}} was meant to allow any type of
validator to be retrieved, irrespective of the way it's implemented (OSGi
service or not). Maybe we should have the same provider logic like you propose
for models.
While the {{Validator}} was meant to validate a single value, multi-value
properties can also be validated. The code has changed a bit since I've donated
it to Sling, but this method \[0\] seems to handle multi-value properties.
For c) I'd stick to returning a boolean value but maybe pass the
ValidationResult object so that the Validator can add its own descriptive
failure messages.
f) sounds cool as it will provide more flexibility if you're thinking to query
all providers for retrieving a model.
\[0\] -
https://github.com/apache/sling/blob/trunk/contrib/extensions/validation/core/src/main/java/org/apache/sling/validation/impl/ValidationServiceImpl.java#L353
Improvement of the validation API
-
Key: SLING-4027
URL: https://issues.apache.org/jira/browse/SLING-4027
Project: Sling
Issue Type: Improvement
Components: Validation
Reporter: Carsten Ziegeler
Assignee: Konrad Windszus
Fix For: Validation 1.0.0
Some comments / thoughts about the validation api:
a) Why is there a validator lookup service? I don't think we need this in the
API - it's a simple OSGI service lookup.
b) A Validator can only validate a single value - what if a property is an
array and the validation needs to validate based on all supplied values? Same
goes with dependencies between two properties?
c) The Validator interface returns null on success and a String (message) if
validation fails. But it can also throw an exception if e.g. the provided
value is null. I think a null value should be treated the same as a wrong
value. Throwing the exception if some configuration like the regexp for the
regexp validator is missing, is fine. but all errors of validating a value
should be treated the same.
d) NonExistingTypeException I don't think we need this -
IllegalArgumentException is fine to throw from the type enumeration
e) Maybe we can also remove the SlingValidationException - it is only thrown
(see c) if a validator does not get its required configuration - which can be
seen as an IllegalStateException
f) It would be nice to have a ValidationModelProvider interface - we will
then have the current way of defining models as the default implememtation.
But can allow other means of defining the validation model
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SLING-4027) Improvement of the validation API
[
https://issues.apache.org/jira/browse/SLING-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14218026#comment-14218026
]
Konrad Windszus commented on SLING-4027:
b) and d) is solved with SLING-4138.
With those changes the validator will never be called with a null value,
therefore I consider c) as being resolved as well (I already adjusted the
javadoc of the {{Validator}} with SLING-4138)
Improvement of the validation API
-
Key: SLING-4027
URL: https://issues.apache.org/jira/browse/SLING-4027
Project: Sling
Issue Type: Improvement
Components: Validation
Reporter: Carsten Ziegeler
Fix For: Validation 1.0.0
Some comments / thoughts about the validation api:
a) Why is there a validator lookup service? I don't think we need this in the
API - it's a simple OSGI service lookup.
b) A Validator can only validate a single value - what if a property is an
array and the validation needs to validate based on all supplied values? Same
goes with dependencies between two properties?
c) The Validator interface returns null on success and a String (message) if
validation fails. But it can also throw an exception if e.g. the provided
value is null. I think a null value should be treated the same as a wrong
value. Throwing the exception if some configuration like the regexp for the
regexp validator is missing, is fine. but all errors of validating a value
should be treated the same.
d) NonExistingTypeException I don't think we need this -
IllegalArgumentException is fine to throw from the type enumeration
e) Maybe we can also remove the SlingValidationException - it is only thrown
(see c) if a validator does not get its required configuration - which can be
seen as an IllegalStateException
f) It would be nice to have a ValidationModelProvider interface - we will
then have the current way of defining models as the default implememtation.
But can allow other means of defining the validation model
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SLING-4027) Improvement of the validation API
[
https://issues.apache.org/jira/browse/SLING-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14218088#comment-14218088
]
Konrad Windszus commented on SLING-4027:
I removed the {{ValidatorLookupService}} in rev 1640589.
Improvement of the validation API
-
Key: SLING-4027
URL: https://issues.apache.org/jira/browse/SLING-4027
Project: Sling
Issue Type: Improvement
Components: Validation
Reporter: Carsten Ziegeler
Fix For: Validation 1.0.0
Some comments / thoughts about the validation api:
a) Why is there a validator lookup service? I don't think we need this in the
API - it's a simple OSGI service lookup.
b) A Validator can only validate a single value - what if a property is an
array and the validation needs to validate based on all supplied values? Same
goes with dependencies between two properties?
c) The Validator interface returns null on success and a String (message) if
validation fails. But it can also throw an exception if e.g. the provided
value is null. I think a null value should be treated the same as a wrong
value. Throwing the exception if some configuration like the regexp for the
regexp validator is missing, is fine. but all errors of validating a value
should be treated the same.
d) NonExistingTypeException I don't think we need this -
IllegalArgumentException is fine to throw from the type enumeration
e) Maybe we can also remove the SlingValidationException - it is only thrown
(see c) if a validator does not get its required configuration - which can be
seen as an IllegalStateException
f) It would be nice to have a ValidationModelProvider interface - we will
then have the current way of defining models as the default implememtation.
But can allow other means of defining the validation model
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SLING-4027) Improvement of the validation API
[ https://issues.apache.org/jira/browse/SLING-4027?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14200070#comment-14200070 ] Konrad Windszus commented on SLING-4027: Some comments about that a) I agree b) That would be nice, but what would that Validator interface then look like? Would you provide the ValueMap as a parameter? I outlined one approach in SLING-4138, but that does not cover multiple property validation. Can you describe how the Validator would look like? c) This depends on b) but basically I think that the validator should never be called with a null value (this is the current status anyhow). Compare also with https://issues.apache.org/jira/browse/SLING-4013. d) I agree. However I would completely remove the Type class as type conversions can be done through the ValueMap already so there is no need for this class IMHO e) SlingValidationException should also be thrown e.g. if an invalid validator is referenced in the JCR. Therefore I think it is useful to be able to distinguish those kind of errors with a dedicated RuntimeException. f) I agree Improvement of the validation API - Key: SLING-4027 URL: https://issues.apache.org/jira/browse/SLING-4027 Project: Sling Issue Type: Improvement Components: Validation Reporter: Carsten Ziegeler Some comments / thoughts about the validation api: a) Why is there a validator lookup service? I don't think we need this in the API - it's a simple OSGI service lookup. b) A Validator can only validate a single value - what if a property is an array and the validation needs to validate based on all supplied values? Same goes with dependencies between two properties? c) The Validator interface returns null on success and a String (message) if validation fails. But it can also throw an exception if e.g. the provided value is null. I think a null value should be treated the same as a wrong value. Throwing the exception if some configuration like the regexp for the regexp validator is missing, is fine. but all errors of validating a value should be treated the same. d) NonExistingTypeException I don't think we need this - IllegalArgumentException is fine to throw from the type enumeration e) Maybe we can also remove the SlingValidationException - it is only thrown (see c) if a validator does not get its required configuration - which can be seen as an IllegalStateException f) It would be nice to have a ValidationModelProvider interface - we will then have the current way of defining models as the default implememtation. But can allow other means of defining the validation model -- This message was sent by Atlassian JIRA (v6.3.4#6332)
