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/25 13:40:48 UTC
[GitHub] [jackrabbit-oak] mreutegg commented on a change in pull request #493: OAK-9695 correctly evaluate property type from definition while check if
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