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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
