You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/02/14 18:01:00 UTC
[GitHub] [jackrabbit-oak] kwin opened a new pull request #493: OAK-9695 correctly evaluate property type from definition while check if
kwin opened a new pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493
protected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] kwin commented on a change in pull request #493: OAK-9695 correctly evaluate property type from definition while check if
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493#discussion_r815341917
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+ assertNotNull(property);
+ property.setValue(false);
+ superuser.save();
+ property.remove();
+ superuser.save();
Review comment:
Done in https://github.com/apache/jackrabbit-oak/pull/493/commits/43443990ba235b9468b1f551e0400318d8561849
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] kwin commented on a change in pull request #493: OAK-9695 correctly evaluate property type from definition while check if
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493#discussion_r815341051
##########
File path: oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
##########
@@ -617,6 +612,19 @@ private Tree findMatchingPropertyDefinition(
}
// Then look through any residual property definitions
+ return findMatchingResidualPropertyDefinition(fuzzyMatch, types, propertyType.isArray(), definedType, undefinedType, exactTypeMatch);
+ }
+
+ private Tree findMatchingResidualPropertyDefinition(Tree fuzzyMatch, List<Tree> types, Type<?> propertyType, boolean exactTypeMatch) {
Review comment:
It is also called from https://github.com/apache/jackrabbit-oak/pull/493/files#diff-75abd61c57624d67f0c501c17199bf4824b5d56a03c22329c1e717c88044aa1cR615 with both arguments having non-predictable values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] kwin merged pull request #493: OAK-9695 correctly evaluate property type from definition while check if
Posted by GitBox <gi...@apache.org>.
kwin merged pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] kwin commented on a change in pull request #493: OAK-9695 correctly evaluate property type from definition while check if
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493#discussion_r815341051
##########
File path: oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
##########
@@ -617,6 +612,19 @@ private Tree findMatchingPropertyDefinition(
}
// Then look through any residual property definitions
+ return findMatchingResidualPropertyDefinition(fuzzyMatch, types, propertyType.isArray(), definedType, undefinedType, exactTypeMatch);
+ }
+
+ private Tree findMatchingResidualPropertyDefinition(Tree fuzzyMatch, List<Tree> types, Type<?> propertyType, boolean exactTypeMatch) {
Review comment:
It is also called from https://github.com/apache/jackrabbit-oak/pull/493/files#diff-75abd61c57624d67f0c501c17199bf4824b5d56a03c22329c1e717c88044aa1cR615 with both arguments having non-predictable values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] mreutegg commented on a change in pull request #493: OAK-9695 correctly evaluate property type from definition while check if
Posted by GitBox <gi...@apache.org>.
mreutegg commented on a change in pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493#discussion_r814739635
##########
File path: oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
##########
@@ -92,7 +92,7 @@ public Status getStatus() {
@Override
public boolean isProtected() throws InvalidItemStateException {
- return getParent().isProtected(name);
+ return getParent().isProtected(name, state.getType());
Review comment:
`state` may be `null` when the property is stale. E.g. another session removed the property.
```suggestion
return getParent().isProtected(name, getPropertyState().getType());
```
The implementation actually has another similar problem. `getParent()` may also return null :-/ I'll create a separate issue for that.
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+ assertNotNull(property);
+ property.setValue(false);
+ superuser.save();
+ property.remove();
+ superuser.save();
+ }
+
+ public void testSetProtectedResidualProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
Review comment:
Same as above.
```suggestion
public void testSetProtectedResidualProperty() throws RepositoryException {
```
##########
File path: oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
##########
@@ -617,6 +612,19 @@ private Tree findMatchingPropertyDefinition(
}
// Then look through any residual property definitions
+ return findMatchingResidualPropertyDefinition(fuzzyMatch, types, propertyType.isArray(), definedType, undefinedType, exactTypeMatch);
+ }
+
+ private Tree findMatchingResidualPropertyDefinition(Tree fuzzyMatch, List<Tree> types, Type<?> propertyType, boolean exactTypeMatch) {
Review comment:
The only call of this method is from here: https://github.com/apache/jackrabbit-oak/pull/493/files#diff-75abd61c57624d67f0c501c17199bf4824b5d56a03c22329c1e717c88044aa1cR215 with `fuzzyMatch` set to `null` and `exactTypeMatch` set to `true`. Those parameters are therefore unnecessary.
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+ assertNotNull(property);
+ property.setValue(false);
+ superuser.save();
+ property.remove();
+ superuser.save();
Review comment:
Please replace tabs with spaces.
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
Review comment:
There is no need to declare `ValueFormatException`, `VersionException`, `LockException`, `ConstraintViolationException`. They are all `RepositoryException`.
```suggestion
public void testSetAndRemoveUnprotectedProperty() throws RepositoryException {
```
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+ assertNotNull(property);
+ property.setValue(false);
+ superuser.save();
+ property.remove();
+ superuser.save();
+ }
+
+ public void testSetProtectedResidualProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Value uriValue = ValueFactoryImpl.getInstance().createValue("http://example.com", PropertyType.URI);
+ try {
+ node.setProperty("test", uriValue);
+ fail("Setting protected property (according to residual propery type definition) must throw ConstraintViolationException");
Review comment:
```suggestion
fail("Setting protected property (according to residual property type definition) must throw ConstraintViolationException");
```
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+ assertNotNull(property);
+ property.setValue(false);
+ superuser.save();
+ property.remove();
+ superuser.save();
+ }
+
+ public void testSetProtectedResidualProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Value uriValue = ValueFactoryImpl.getInstance().createValue("http://example.com", PropertyType.URI);
+ try {
+ node.setProperty("test", uriValue);
+ fail("Setting protected property (according to residual propery type definition) must throw ConstraintViolationException");
+ } catch (ConstraintViolationException e) {
Review comment:
Please replace tabs with spaces.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] kwin commented on a change in pull request #493: OAK-9695 correctly evaluate property type from definition while check if
Posted by GitBox <gi...@apache.org>.
kwin commented on a change in pull request #493:
URL: https://github.com/apache/jackrabbit-oak/pull/493#discussion_r815341908
##########
File path: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
##########
@@ -274,4 +285,23 @@ public void testPropertyIteratorSize() throws Exception{
assertEquals(3, pitr.getSize());
assertEquals(3, Iterators.size(pitr));
}
+
+ public void testSetAndRemoveUnprotectedProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+ assertNotNull(property);
+ property.setValue(false);
+ superuser.save();
+ property.remove();
+ superuser.save();
+ }
+
+ public void testSetProtectedResidualProperty() throws ValueFormatException, VersionException, LockException, ConstraintViolationException, RepositoryException {
+ Value uriValue = ValueFactoryImpl.getInstance().createValue("http://example.com", PropertyType.URI);
+ try {
+ node.setProperty("test", uriValue);
+ fail("Setting protected property (according to residual propery type definition) must throw ConstraintViolationException");
+ } catch (ConstraintViolationException e) {
Review comment:
Done in https://github.com/apache/jackrabbit-oak/pull/493/commits/43443990ba235b9468b1f551e0400318d8561849
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org