[ https://issues.apache.org/jira/browse/OAK-2828?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661826#comment-14661826 ]
Alex Parvulescu commented on OAK-2828:
--------------------------------------
I think the patch looks good, but I don't have a very good idea yet on why you need to do this, so a few questions:
* you mention needing to set/reset potential components but you use Lists and not Sets, so this makes it possible to add the same _EditorProvider_ instance multiple times by accident.
* about the reseting part: there's no option of removing anything, so you're only basically adding new components, unless (see my next point):
* I'm not too happy about this pattern:
{code}
if ( editorProviders.isEmpty() ) {
editorProviders.add(new ItemSaveValidatorProvider());
editorProviders.add(new NameValidatorProvider());
editorProviders.add(new NamespaceEditorProvider());
editorProviders.add(new TypeEditorProvider());
editorProviders.add(new ConflictValidatorProvider());
editorProviders.add(new AtomicCounterEditorProvider());
}
{code}
this means that you either start with the defaults _or_ have to wire every existing implementation of specific editors yourself, which is very brittle (I could not name them all if you ask me).
I'm working with the assumption that you're not looking to remove any OOTB editors, as they are all very important, so what is missing from your POV here?
The way I see it, I'm in favor of lazy inits, this will also provide the option to add custom stuff (possibly also remove it if you really need it), but the current patch makes too many assumptions, I would simplify it a bit and work from there to provide still missing features.
> Jcr builder class does not allow overriding most of its dependencies
> --------------------------------------------------------------------
>
> Key: OAK-2828
> URL: https://issues.apache.org/jira/browse/OAK-2828
> Project: Jackrabbit Oak
> Issue Type: Sub-task
> Components: jcr
> Affects Versions: 1.2.2
> Reporter: Robert Munteanu
> Assignee: Francesco Mari
> Labels: modularization, technical_debt
> Fix For: 1.3.5
>
> Attachments: 0001-OAK-2828-Jcr-builder-class-does-not-allow-overriding.patch
>
>
> The {{Jcr}} class is the entry point for configuring a JCR repository using an Oak backend. However, it always use a hardcoded set of dependencies ( IndexEditorProvider, SecurityProvider, etc ) which cannot be reset, as they are defined in the constructor and the builder {{with}} methods eagerly configure the backing {{Oak}} instance with those dependencies.
> As an example
> {code:java|title=Jcr.java}
> @Nonnull
> public final Jcr with(@Nonnull SecurityProvider securityProvider) {
> oak.with(checkNotNull(securityProvider));
> this.securityProvider = securityProvider;
> return this;
> }
> {code}
> injects the security provider which in turn starts configuring the Oak repository provider
> {code:java|title=Oak.java}
> @Nonnull
> public Oak with(@Nonnull SecurityProvider securityProvider) {
> this.securityProvider = checkNotNull(securityProvider);
> if (securityProvider instanceof WhiteboardAware) {
> ((WhiteboardAware) securityProvider).setWhiteboard(whiteboard);
> }
> for (SecurityConfiguration sc : securityProvider.getConfigurations()) {
> RepositoryInitializer ri = sc.getRepositoryInitializer();
> if (ri != RepositoryInitializer.DEFAULT) {
> initializers.add(ri);
> }
> }
> return this;
> }
> {code}
> Instead, the {{Jcr}} class should store the configured dependencies and only configure the {{Oak}} instance when {{createRepository}} is invoked.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)