You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Marcel Reutegger <mr...@adobe.com> on 2012/10/22 14:56:32 UTC
RE: svn commit: r1400105 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
oak-run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
Hi,
I think this is the wrong place for the OrderedChildrenEditor. This editor
is required for oak-core to work correctly. It's not an optional or JCR
specific editor.
Regards
Marcel
> -----Original Message-----
> From: jukka@apache.org [mailto:jukka@apache.org]
> Sent: Freitag, 19. Oktober 2012 16:13
> To: oak-commits@jackrabbit.apache.org
> Subject: svn commit: r1400105 - in /jackrabbit/oak/trunk: oak-
> core/src/main/java/org/apache/jackrabbit/oak/Oak.java oak-
> jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java oak-
> run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
>
> Author: jukka
> Date: Fri Oct 19 14:13:18 2012
> New Revision: 1400105
>
> URL: http://svn.apache.org/viewvc?rev=1400105&view=rev
> Log:
> OAK-352: Oak builder for simplified repository construction
>
> Address remaining FIXMEs, un-hardcode the OrderedChildEditor.
>
> Modified:
> jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/Oak.java
> jackrabbit/oak/trunk/oak-
> jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
> jackrabbit/oak/trunk/oak-
> run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
>
> Modified: jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/Oak.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/Oak.java?rev=1400105&r1=1
> 400104&r2=1400105&view=diff
> ==========================================================
> ====================
> --- jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/Oak.java (original)
> +++ jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/Oak.java Fri Oct 19 14:13:18
> 2012
> @@ -30,7 +30,6 @@ import org.apache.jackrabbit.oak.api.Con
> import org.apache.jackrabbit.oak.api.ContentSession;
> import org.apache.jackrabbit.oak.api.Root;
> import org.apache.jackrabbit.oak.core.ContentRepositoryImpl;
> -import org.apache.jackrabbit.oak.core.OrderedChildrenEditor;
> import org.apache.jackrabbit.oak.kernel.KernelNodeStore;
> import org.apache.jackrabbit.oak.spi.commit.CommitHook;
> import org.apache.jackrabbit.oak.spi.commit.CompositeHook;
> @@ -68,7 +67,7 @@ public class Oak {
>
> private final List<CommitHook> commitHooks = Lists.newArrayList();
>
> - private final List<ValidatorProvider> validatorProviders =
> Lists.newArrayList();
> + private List<ValidatorProvider> validatorProviders = Lists.newArrayList();
>
> private SecurityProvider securityProvider = new OpenSecurityProvider();
>
> @@ -126,7 +125,7 @@ public class Oak {
> if (!validatorProviders.isEmpty()) {
> commitHooks.add(new ValidatingHook(
> CompositeValidatorProvider.compose(validatorProviders)));
> - //validatorProviders.clear(); FIXME
> + validatorProviders = Lists.newArrayList();
> }
> }
>
> @@ -163,7 +162,6 @@ public class Oak {
> @Nonnull
> public Oak with(@Nonnull SecurityProvider securityProvider) {
> this.securityProvider = securityProvider;
> -
> try {
>
> validatorProviders.addAll(securityProvider.getAccessControlProvider().getVa
> lidatorProviders());
>
> validatorProviders.addAll(securityProvider.getUserConfiguration().getValidat
> orProviders());
> @@ -191,7 +189,10 @@ public class Oak {
> for (MicroKernelTracker initializer : initializers) {
> initializer.available(store);
> }
> - store.setHook(createCommitHook());
> +
> + withValidatorHook();
> + store.setHook(CompositeHook.compose(commitHooks));
> +
> return new ContentRepositoryImpl(
> store,
> conflictHandler,
> @@ -247,10 +248,4 @@ public class Oak {
> return createContentSession().getLatestRoot();
> }
>
> - private CommitHook createCommitHook() {
> - withValidatorHook();
> - commitHooks.add(new OrderedChildrenEditor()); // FIXME don't
> hardcode
> - return CompositeHook.compose(commitHooks);
> - }
> -
> }
> \ No newline at end of file
>
> Modified: jackrabbit/oak/trunk/oak-
> jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-
> jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java?rev=1
> 400105&r1=1400104&r2=1400105&view=diff
> ==========================================================
> ====================
> --- jackrabbit/oak/trunk/oak-
> jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
> (original)
> +++ jackrabbit/oak/trunk/oak-
> jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java Fri Oct
> 19 14:13:18 2012
> @@ -30,6 +30,7 @@ import org.apache.jackrabbit.mk.api.Micr
> import org.apache.jackrabbit.oak.Oak;
> import org.apache.jackrabbit.oak.api.ContentRepository;
> import org.apache.jackrabbit.oak.api.ContentSession;
> +import org.apache.jackrabbit.oak.core.OrderedChildrenEditor;
> import org.apache.jackrabbit.oak.kernel.KernelNodeStore;
> import
> org.apache.jackrabbit.oak.plugins.commit.AnnotatingConflictHandler;
> import org.apache.jackrabbit.oak.plugins.commit.ConflictValidatorProvider;
> @@ -67,7 +68,8 @@ public class RepositoryImpl implements R
> private static final CompositeHook DEFAULT_COMMIT_HOOK =
> new CompositeHook(
> new ValidatingHook(DEFAULT_VALIDATOR),
> - new PropertyIndexHook());
> + new PropertyIndexHook(),
> + new OrderedChildrenEditor());
>
> private static final ConflictHandler DEFAULT_CONFLICT_HANDLER = new
> AnnotatingConflictHandler();
>
>
> Modified: jackrabbit/oak/trunk/oak-
> run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-
> run/src/main/java/org/apache/jackrabbit/oak/run/Main.java?rev=1400105&
> r1=1400104&r2=1400105&view=diff
> ==========================================================
> ====================
> --- jackrabbit/oak/trunk/oak-
> run/src/main/java/org/apache/jackrabbit/oak/run/Main.java (original)
> +++ jackrabbit/oak/trunk/oak-
> run/src/main/java/org/apache/jackrabbit/oak/run/Main.java Fri Oct 19
> 14:13:18 2012
> @@ -25,6 +25,7 @@ import org.apache.jackrabbit.mk.api.Micr
> import org.apache.jackrabbit.mk.core.MicroKernelImpl;
> import org.apache.jackrabbit.oak.Oak;
> import org.apache.jackrabbit.oak.api.ContentRepository;
> +import org.apache.jackrabbit.oak.core.OrderedChildrenEditor;
> import org.apache.jackrabbit.oak.http.OakServlet;
> import org.apache.jackrabbit.oak.jcr.RepositoryImpl;
> import org.apache.jackrabbit.oak.plugins.commit.ConflictValidatorProvider;
> @@ -206,7 +207,8 @@ public class Main {
> new ValidatingHook(createDefaultValidatorProvider()),
> new PropertyIndexHook(),
> new LuceneReindexHook(DEFAULT_INDEX_HOME),
> - new LuceneHook(DEFAULT_INDEX_HOME));
> + new LuceneHook(DEFAULT_INDEX_HOME),
> + new OrderedChildrenEditor());
> }
>
> private static ValidatorProvider createDefaultValidatorProvider() {
>
Re: svn commit: r1400105 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
oak-run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, Oct 22, 2012 at 3:53 PM, Marcel Reutegger <mr...@adobe.com> wrote:
> why can't it be both? It used to be hardcoded in oak-core
> before you moved it to RepositoryImpl...
Two reasons:
1) To avoid confusion. So far all our plugins are such that they can
be selectively included or excluded depending on the kind of a
deployment.
2) For performance. The OCE does a diff over the whole change set to
see if ordering changes are needed. This shouldn't be needed in
deployments where we don't care about ordering. In such deployments
IMHO only explicit orderBefore (and possibly re/move) calls should
need to deal with ordering.
BR,
Jukka Zitting
RE: svn commit: r1400105 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
oak-run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
Posted by Marcel Reutegger <mr...@adobe.com>.
> OK. If the remaining functionality is something that can't or
> shouldn't be removed, then I'd rather see it hardcoded in
> Root/TreeImpl than implemented as a plugin.
why can't it be both? It used to be hardcoded in oak-core
before you moved it to RepositoryImpl...
Regards
Marcel
Re: svn commit: r1400105 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
oak-run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, Oct 22, 2012 at 3:24 PM, Marcel Reutegger <mr...@adobe.com> wrote:
> I'm currently working on the copy/move issue. After that we may
> not need the OCE anymore.
OK. If the remaining functionality is something that can't or
shouldn't be removed, then I'd rather see it hardcoded in
Root/TreeImpl than implemented as a plugin.
BR,
Jukka Zitting
RE: svn commit: r1400105 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
oak-run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
Posted by Marcel Reutegger <mr...@adobe.com>.
> On Mon, Oct 22, 2012 at 2:56 PM, Marcel Reutegger
> <mr...@adobe.com> wrote:
> > I think this is the wrong place for the OrderedChildrenEditor. This editor
> > is required for oak-core to work correctly. It's not an optional or JCR
> > specific editor.
>
> Hmm, what will fail if the OCE is not in place?
The two tests in RootTest, which are currently ignored for a related
reason. The purpose of the OCE is to make sure all children are
present in the hidden :childOrder property. This is rather a safety
net and shoulnd't be required. The other purpose is to maintain
the :childOrder property on commit when Root.copy() or Root.move()
was called. See also: https://issues.apache.org/jira/browse/OAK-169?focusedCommentId=13474153&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13474153
I'm currently working on the copy/move issue. After that we may
not need the OCE anymore.
Regards
Marcel
Re: svn commit: r1400105 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
oak-run/src/main/java/org/apache/jackrabbit/oak/run/Main.java
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, Oct 22, 2012 at 2:56 PM, Marcel Reutegger <mr...@adobe.com> wrote:
> I think this is the wrong place for the OrderedChildrenEditor. This editor
> is required for oak-core to work correctly. It's not an optional or JCR
> specific editor.
Hmm, what will fail if the OCE is not in place?
BR,
Jukka Zitting