You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Felix Meschberger <fm...@gmail.com> on 2010/06/07 15:46:46 UTC

Namespace and Node Type registration

Hi all,

I am looking at a strange situation starting Sling Trunk build. Not all
namespace prefixes seem to be registered in the Loader class of the JCR
Base bundle.

IMHO the setup in the JCR Base Bundle between the
AbstractSlingRepository and the Loader is not optimal:

  * AbstractSlingRepository implements SynchronousBundleListener
     to just forward the events to the Loader
  * There is a time gap between the time the Loader class is
     instantiated (and handling a set of bundles) and the time
     AbstractSlingRepository is registered as a bundle listener
  * There is a bug in the Loader constructor preventing all
     just INSTALLED bundles from being registered by the BundleListener
     only handles the INSTALLED (and UNINSTALLED and UPDATED) events.

The last part will certainly miss some bundles.

I suggest we change this setup such, that the Loader itself is the
bundle listener handles the events itself. The AbstractSlingRepository
just sets up the loader and uses it.

See patch in SLING-1548.

WDYT ?

Regards
Felix

[1] https://issues.apache.org/jira/browse/SLING-1548

Re: Namespace and Node Type registration

Posted by Carsten Ziegeler <cz...@apache.org>.
Felix Meschberger  wrote

> I agree with the name "Loader" not being a good one.
> 
> Currently I am working on a path along these lines:
> 
>   * Split namespace and node type handling in two classes
>   * BundleNameSpaceMapper is a DS Component providing the
>      NamespaceMapper service reading maapings from the manifest
>      This is independent of any repository; and there is just
>      one instance per framework.
>   * NodeTypeRegistrar is managed by the AbstractSlingRepository,
>      it is related to the repository which controls it and
>      just manages node types and there is an instance for each
>      SlingRepository service.
> 
> The NodeTypeRegistrar should be the basis to enhance it to implement
> SLING-7.
> 
> I think both should keep on living in the JCR Base bundle.
> 
> WDYT ?
+1

Carsten

-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Namespace and Node Type registration

Posted by Justin Edelson <ju...@gmail.com>.
On 6/7/10 3:05 PM, Felix Meschberger wrote:
> Hi,
> 
> On 07.06.2010 16:56, Justin Edelson wrote:
>> On 6/7/10 10:45 AM, Carsten Ziegeler wrote:
>>> Justin Edelson  wrote
>>>> And then move it to jcr.contentloader?
>>> I think while contentloader is more an optional bundle, the namespace
>>> prefix handling is more a required thing, therefore I would not move
>>> this to the contentloader.
>> I hadn't thought of contentloader being optional. In that context, I
>> agree that nodetype and prefix loading is not a good match.
>>
>> I just find having classes named *Loader which aren't in the bundle
>> called contentloader is non-intuitive. I think I still look in
>> contentloader when I want to debug node type loading :)
>>
>> But loss of a few minutes every month isn't worth a breaking change :)
>>
> 
> I agree with the name "Loader" not being a good one.
> 
> Currently I am working on a path along these lines:
> 
>   * Split namespace and node type handling in two classes
>   * BundleNameSpaceMapper is a DS Component providing the
>      NamespaceMapper service reading maapings from the manifest
>      This is independent of any repository; and there is just
>      one instance per framework.
>   * NodeTypeRegistrar is managed by the AbstractSlingRepository,
>      it is related to the repository which controls it and
>      just manages node types and there is an instance for each
>      SlingRepository service.
> 
> The NodeTypeRegistrar should be the basis to enhance it to implement
> SLING-7.
> 
> I think both should keep on living in the JCR Base bundle.
> 
> WDYT ?
+1

Justin

> 
> Regards
> Felix



Re: Namespace and Node Type registration

Posted by Felix Meschberger <fm...@gmail.com>.
Hi,

On 07.06.2010 16:56, Justin Edelson wrote:
> On 6/7/10 10:45 AM, Carsten Ziegeler wrote:
>> Justin Edelson  wrote
>>> And then move it to jcr.contentloader?
>> I think while contentloader is more an optional bundle, the namespace
>> prefix handling is more a required thing, therefore I would not move
>> this to the contentloader.
> I hadn't thought of contentloader being optional. In that context, I
> agree that nodetype and prefix loading is not a good match.
> 
> I just find having classes named *Loader which aren't in the bundle
> called contentloader is non-intuitive. I think I still look in
> contentloader when I want to debug node type loading :)
> 
> But loss of a few minutes every month isn't worth a breaking change :)
> 

I agree with the name "Loader" not being a good one.

Currently I am working on a path along these lines:

  * Split namespace and node type handling in two classes
  * BundleNameSpaceMapper is a DS Component providing the
     NamespaceMapper service reading maapings from the manifest
     This is independent of any repository; and there is just
     one instance per framework.
  * NodeTypeRegistrar is managed by the AbstractSlingRepository,
     it is related to the repository which controls it and
     just manages node types and there is an instance for each
     SlingRepository service.

The NodeTypeRegistrar should be the basis to enhance it to implement
SLING-7.

I think both should keep on living in the JCR Base bundle.

WDYT ?

Regards
Felix

Re: Namespace and Node Type registration

Posted by Justin Edelson <ju...@gmail.com>.
On 6/7/10 10:45 AM, Carsten Ziegeler wrote:
> Justin Edelson  wrote
>> And then move it to jcr.contentloader?
> I think while contentloader is more an optional bundle, the namespace
> prefix handling is more a required thing, therefore I would not move
> this to the contentloader.
I hadn't thought of contentloader being optional. In that context, I
agree that nodetype and prefix loading is not a good match.

I just find having classes named *Loader which aren't in the bundle
called contentloader is non-intuitive. I think I still look in
contentloader when I want to debug node type loading :)

But loss of a few minutes every month isn't worth a breaking change :)

Justin

> 
> Carsten


Re: Namespace and Node Type registration

Posted by Carsten Ziegeler <cz...@apache.org>.
The patch looks good to me and the proposed changes make sense.

Justin Edelson  wrote
> Could we go a step further and have Loader be a standalone DS component
> which just references SlingRepository? 
Yes, I think making this a standalone component might make even more sense.


> And then move it to jcr.contentloader?
I think while contentloader is more an optional bundle, the namespace
prefix handling is more a required thing, therefore I would not move
this to the contentloader.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: Namespace and Node Type registration

Posted by Justin Edelson <ju...@gmail.com>.
Could we go a step further and have Loader be a standalone DS component
which just references SlingRepository? And then move it to
jcr.contentloader?

There doesn't appear to be a reason for depending upon the concrete
class AbstractSlingRepository instead of the SlingRepository interface.
I think there might have been pre-Jackrabbit 2, but there isn't now.

Justin


On 6/7/10 9:46 AM, Felix Meschberger wrote:
> Hi all,
> 
> I am looking at a strange situation starting Sling Trunk build. Not all
> namespace prefixes seem to be registered in the Loader class of the JCR
> Base bundle.
> 
> IMHO the setup in the JCR Base Bundle between the
> AbstractSlingRepository and the Loader is not optimal:
> 
>   * AbstractSlingRepository implements SynchronousBundleListener
>      to just forward the events to the Loader
>   * There is a time gap between the time the Loader class is
>      instantiated (and handling a set of bundles) and the time
>      AbstractSlingRepository is registered as a bundle listener
>   * There is a bug in the Loader constructor preventing all
>      just INSTALLED bundles from being registered by the BundleListener
>      only handles the INSTALLED (and UNINSTALLED and UPDATED) events.
> 
> The last part will certainly miss some bundles.
> 
> I suggest we change this setup such, that the Loader itself is the
> bundle listener handles the events itself. The AbstractSlingRepository
> just sets up the loader and uses it.
> 
> See patch in SLING-1548.
> 
> WDYT ?
> 
> Regards
> Felix
> 
> [1] https://issues.apache.org/jira/browse/SLING-1548