[jira] [Assigned] (OAK-285) Split CommitEditor into CommitEditor and Validator interfaces
[
https://issues.apache.org/jira/browse/OAK-285?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Dürig reassigned OAK-285:
-
Assignee: Michael Dürig
> Split CommitEditor into CommitEditor and Validator interfaces
> -
>
> Key: OAK-285
> URL: https://issues.apache.org/jira/browse/OAK-285
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
>Reporter: Michael Dürig
>Assignee: Michael Dürig
>
> Having {{ValidatingEditor}} extend {{CommitEditor}} is a constant source of
> confusion since the former does not edit at all. Since it also makes sense to
> run validation only after all editing has taken place, I suggest to introduce
> a new interface {{CommitValidator}} which covers the validating aspect of a
> commit. Alternatively, we could of course also re-use the {{Observer}}
> interface here.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-285) Split CommitEditor into CommitEditor and Validator interfaces
[
https://issues.apache.org/jira/browse/OAK-285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443918#comment-13443918
]
Michael Dürig commented on OAK-285:
---
bq. That's easy to control by managing the order of editors in a
CompositeEditor instance.
That's what I wanted to avoid as I have the feeling this might be too brittle.
Splitting it up in the way I proposed might not be fine grained enough though.
So I agree it is best to leave it as is until we have a better grasp on what we
actually need here.
bq. How about renaming CommitEditor to a more generic CommitHook instead?
+1
> Split CommitEditor into CommitEditor and Validator interfaces
> -
>
> Key: OAK-285
> URL: https://issues.apache.org/jira/browse/OAK-285
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
>Reporter: Michael Dürig
>
> Having {{ValidatingEditor}} extend {{CommitEditor}} is a constant source of
> confusion since the former does not edit at all. Since it also makes sense to
> run validation only after all editing has taken place, I suggest to introduce
> a new interface {{CommitValidator}} which covers the validating aspect of a
> commit. Alternatively, we could of course also re-use the {{Observer}}
> interface here.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: Inconsistent locking in TreeImpl.Children?
Hi, On 28.8.12 16:37, Marcel Reutegger wrote: TreeImpl.Children.iterator() returns an iterator over the values of the internal children Map without locking. Other methods accessing the children Map use read or write locks, depending on the operation. I'm wondering if locking is missing there or if it is even needed. The thinking is that the iterators returned from Tree have snapshot semantics. That is, they represent the state of the underlying tree at the point the iterator was returned. Later changes are not reflected nor do mutating calls to the tree from within such an iterator result in a ConcurrentModificationException. See the Javadoc for Tree.getChildren. While the initial design might have deteriorated to some degree I think the general contract still holds. Furthermore locking should not be necessary since by contract Tree instances are not thread safe. BTW. Children.iterator is currently never called at all. It is a leftover from an earlier implementation where the caller ensured the contract described above would hold. Michael
[jira] [Created] (OAK-286) Possible NPE in LuceneIndex
Tommaso Teofili created OAK-286: --- Summary: Possible NPE in LuceneIndex Key: OAK-286 URL: https://issues.apache.org/jira/browse/OAK-286 Project: Jackrabbit Oak Issue Type: Bug Components: core Affects Versions: 0.4 Reporter: Tommaso Teofili Priority: Trivial Fix For: 0.5 in LuceneIndex#getQuery(Filter) the first variable is initialized to null and updated to a meaningful value only if pr.first is not null, thus it may result in NPE when calling first.equals(last) (line 168). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (OAK-286) Possible NPE in LuceneIndex
[ https://issues.apache.org/jira/browse/OAK-286?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Tommaso Teofili updated OAK-286: Attachment: OAK-286.patch trivial patch which checks for first being not null > Possible NPE in LuceneIndex > --- > > Key: OAK-286 > URL: https://issues.apache.org/jira/browse/OAK-286 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 0.4 >Reporter: Tommaso Teofili >Priority: Trivial > Fix For: 0.5 > > Attachments: OAK-286.patch > > > in LuceneIndex#getQuery(Filter) the first variable is initialized to null and > updated to a meaningful value only if pr.first is not null, thus it may > result in NPE when calling first.equals(last) (line 168). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Created] (OAK-287) PrivilegeManagerImplTest.testJcrAll assumes that there are no custom privileges
Jukka Zitting created OAK-287:
-
Summary: PrivilegeManagerImplTest.testJcrAll assumes that there
are no custom privileges
Key: OAK-287
URL: https://issues.apache.org/jira/browse/OAK-287
Project: Jackrabbit Oak
Issue Type: Bug
Components: jcr
Reporter: Jukka Zitting
Priority: Minor
The {{testJcrAll}} test checks the contents of the {{jcr:all}} privilege by
first verifying that all standard JCR privileges are included and then
asserting that no other privileges are included.
That last assertion doesn't work properly since the {{jcr:all}} privilege is
defined to also "include all implementation-defined privileges" and in some
orderings of the test suite the other test cases like
{{testRegisterCustomPrivileges}} have already added such custom privileges
before the {{testJcrAll}} test gets executed.
Ideally we'd run each test case in a completely isolated state so the ordering
of tests won't matter. If that can't be done, the assertion at the end of
{{testJcrAll}} should be disabled.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Comment Edited] (OAK-286) Possible NPE in LuceneIndex
[
https://issues.apache.org/jira/browse/OAK-286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443952#comment-13443952
]
Alex Parvulescu edited comment on OAK-286 at 8/29/12 9:52 PM:
--
good point, but is taking out the entire Query creation a good idea?
what if {{first}} is null and {{last}} is not? we still need to have the
{{TermRangeQuery.newStringRange}} created with an upper bound - lucene seems to
allow this.
so I'd move the null check as a first condition on the if:
{{if (first.equals(last) && pr.firstIncluding && pr.lastIncluding)}}
becomes
{{if (first !=null && first.equals(last) && pr.firstIncluding &&
pr.lastIncluding)}}
[edit] tweaked formatting a bit
was (Author: alex.parvulescu):
good point, but is taking out the entire Query creation a good idea?
what if first is null and last it not? we still need to have the
TermRangeQuery.newStringRange created with an upper bound - lucene seems to
allow this.
so I'd move the null check as a first condition on the if:
{{if (first.equals(last) && pr.firstIncluding && pr.lastIncluding)}}
becomes
{{if (first !=null && first.equals(last) && pr.firstIncluding &&
pr.lastIncluding)}}
> Possible NPE in LuceneIndex
> ---
>
> Key: OAK-286
> URL: https://issues.apache.org/jira/browse/OAK-286
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: core
>Affects Versions: 0.4
>Reporter: Tommaso Teofili
>Priority: Trivial
> Fix For: 0.5
>
> Attachments: OAK-286.patch
>
>
> in LuceneIndex#getQuery(Filter) the first variable is initialized to null and
> updated to a meaningful value only if pr.first is not null, thus it may
> result in NPE when calling first.equals(last) (line 168).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OAK-286) Possible NPE in LuceneIndex
[
https://issues.apache.org/jira/browse/OAK-286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443952#comment-13443952
]
Alex Parvulescu commented on OAK-286:
-
good point, but is taking out the entire Query creation a good idea?
what if first is null and last it not? we still need to have the
TermRangeQuery.newStringRange created with an upper bound - lucene seems to
allow this.
so I'd move the null check as a first condition on the if:
{{if (first.equals(last) && pr.firstIncluding && pr.lastIncluding)}}
becomes
{{if (first !=null && first.equals(last) && pr.firstIncluding &&
pr.lastIncluding)}}
> Possible NPE in LuceneIndex
> ---
>
> Key: OAK-286
> URL: https://issues.apache.org/jira/browse/OAK-286
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: core
>Affects Versions: 0.4
>Reporter: Tommaso Teofili
>Priority: Trivial
> Fix For: 0.5
>
> Attachments: OAK-286.patch
>
>
> in LuceneIndex#getQuery(Filter) the first variable is initialized to null and
> updated to a meaningful value only if pr.first is not null, thus it may
> result in NPE when calling first.equals(last) (line 168).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (OAK-285) Split CommitEditor into CommitEditor and Validator interfaces
[
https://issues.apache.org/jira/browse/OAK-285?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Dürig resolved OAK-285.
---
Resolution: Fixed
Fix Version/s: 0.5
Fixed at revision 1378491
> Split CommitEditor into CommitEditor and Validator interfaces
> -
>
> Key: OAK-285
> URL: https://issues.apache.org/jira/browse/OAK-285
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
>Reporter: Michael Dürig
>Assignee: Michael Dürig
> Fix For: 0.5
>
>
> Having {{ValidatingEditor}} extend {{CommitEditor}} is a constant source of
> confusion since the former does not edit at all. Since it also makes sense to
> run validation only after all editing has taken place, I suggest to introduce
> a new interface {{CommitValidator}} which covers the validating aspect of a
> commit. Alternatively, we could of course also re-use the {{Observer}}
> interface here.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (OAK-287) PrivilegeManagerImplTest.testJcrAll assumes that there are no custom privileges
[
https://issues.apache.org/jira/browse/OAK-287?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jukka Zitting resolved OAK-287.
---
Resolution: Fixed
Fix Version/s: 0.5
Assignee: Jukka Zitting
Fixed in revision 1378510 by making each test case to use a separate fresh
repository.
> PrivilegeManagerImplTest.testJcrAll assumes that there are no custom
> privileges
> ---
>
> Key: OAK-287
> URL: https://issues.apache.org/jira/browse/OAK-287
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: jcr
>Reporter: Jukka Zitting
>Assignee: Jukka Zitting
>Priority: Minor
> Fix For: 0.5
>
>
> The {{testJcrAll}} test checks the contents of the {{jcr:all}} privilege by
> first verifying that all standard JCR privileges are included and then
> asserting that no other privileges are included.
> That last assertion doesn't work properly since the {{jcr:all}} privilege is
> defined to also "include all implementation-defined privileges" and in some
> orderings of the test suite the other test cases like
> {{testRegisterCustomPrivileges}} have already added such custom privileges
> before the {{testJcrAll}} test gets executed.
> Ideally we'd run each test case in a completely isolated state so the
> ordering of tests won't matter. If that can't be done, the assertion at the
> end of {{testJcrAll}} should be disabled.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
buildbot failure in ASF Buildbot on oak-trunk
The Buildbot has detected a new failure on builder oak-trunk while building ASF Buildbot. Full details are available at: http://ci.apache.org/builders/oak-trunk/builds/454 Buildbot URL: http://ci.apache.org/ Buildslave for this Build: osiris_ubuntu Build Reason: scheduler Build Source Stamp: [branch jackrabbit/oak/trunk] 1378510 Blamelist: jukka BUILD FAILED: failed compile sincerely, -The Buildbot
buildbot success in ASF Buildbot on oak-trunk
The Buildbot has detected a restored build on builder oak-trunk while building ASF Buildbot. Full details are available at: http://ci.apache.org/builders/oak-trunk/builds/455 Buildbot URL: http://ci.apache.org/ Buildslave for this Build: osiris_ubuntu Build Reason: scheduler Build Source Stamp: [branch jackrabbit/oak/trunk] 1378514 Blamelist: mduerig Build succeeded! sincerely, -The Buildbot
[jira] [Commented] (OAK-286) Possible NPE in LuceneIndex
[ https://issues.apache.org/jira/browse/OAK-286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444006#comment-13444006 ] Tommaso Teofili commented on OAK-286: - I agree with you Alex, your solution looks ok as null lower bound is allowed (actually null for upper is allowed too). > Possible NPE in LuceneIndex > --- > > Key: OAK-286 > URL: https://issues.apache.org/jira/browse/OAK-286 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 0.4 >Reporter: Tommaso Teofili >Priority: Trivial > Fix For: 0.5 > > Attachments: OAK-286.patch > > > in LuceneIndex#getQuery(Filter) the first variable is initialized to null and > updated to a meaningful value only if pr.first is not null, thus it may > result in NPE when calling first.equals(last) (line 168). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: svn commit: r1378514 - /jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/privilege/PrivilegeManagerImplTest.java
Hi, On Wed, Aug 29, 2012 at 2:09 PM, wrote: > OAK-287: PrivilegeManagerImplTest.testJcrAll assumes that there are no custom > privileges > resolve merge conflict with changes from OAK-285 Thanks! BR, Jukka Zitting
[jira] [Resolved] (OAK-286) Possible NPE in LuceneIndex
[ https://issues.apache.org/jira/browse/OAK-286?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Alex Parvulescu resolved OAK-286. - Resolution: Fixed thanks Tommaso! fixed with revision 1378643. > Possible NPE in LuceneIndex > --- > > Key: OAK-286 > URL: https://issues.apache.org/jira/browse/OAK-286 > Project: Jackrabbit Oak > Issue Type: Bug > Components: core >Affects Versions: 0.4 >Reporter: Tommaso Teofili >Priority: Trivial > Fix For: 0.5 > > Attachments: OAK-286.patch > > > in LuceneIndex#getQuery(Filter) the first variable is initialized to null and > updated to a meaningful value only if pr.first is not null, thus it may > result in NPE when calling first.equals(last) (line 168). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
