You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by Jukka Zitting <ju...@gmail.com> on 2010/05/17 10:43:40 UTC
FYI: Moving session-related classes to o.a.j.core.session
Hi,
As a part of my work on JCR-890, I'm planning to move most of the
session-related classes from o.a.j.core to a new o.a.j.core.session
package. This will make it easier to review and control related
dependencies and code paths, and to ultimately guard them against
access from concurrent threads.
As the first step I'm simply moving the relevant classes and making
the minor dependency changes where needed, so the functional risk
should be low. However, the moves will likely invalidate many other
pending jackrabbit-core changes, so please let me know if you have
pending changes that I should wait for before I move these classes.
Unless there's a need to wait, I'm planning to commit the changes in
the afternoon today.
BR,
Jukka Zitting
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, May 17, 2010 at 3:05 PM, Jukka Zitting <ju...@gmail.com> wrote:
> I'll prepare a patch and outline the steps by which I plan to address
> the issues raised here. I'll commit the changes only when everyone is
> happy with the approach.
See JCR-890 for the proposed patch and a more detailed explanation of
the changes.
BR,
Jukka Zitting
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, May 17, 2010 at 10:43 AM, Jukka Zitting <ju...@gmail.com> wrote:
> Unless there's a need to wait, I'm planning to commit the changes in
> the afternoon today.
This obviously needs more time than I anticipated. I'm sorry about the rush.
I'll prepare a patch and outline the steps by which I plan to address
the issues raised here. I'll commit the changes only when everyone is
happy with the approach.
BR,
Jukka Zitting
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Angela Schreiber <an...@day.com>.
Stefan Guggisberg wrote:
> i share tomas's concerns.
so do i... in addition i have quite some changes
pending (JCR-2573, JCR-2629), which are not yet ready for commit.
>> UserPerWorkspaceSecurityManager
>> (>security), DefaultSecurityManager (>security),
that doesn't work without major changes.
those classes are not in the core package because i thought they
belong there, but rather because they need access to non-public
methods and classes (at least one has a TODO regarding this).
>> ItemValidator (>nodetype).
the ItemValidator is not specific for node types. it does
all kind of validation checks including access control...
it was extracted from the batchitemoperation stuff that - as far
as i know - was placed in the core package for the same reasons
as mentioned above.
-1 for moving to the nodetype package.
regards
angela
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Stefan Guggisberg <st...@day.com>.
On Mon, May 17, 2010 at 11:00 AM, Thomas Müller <th...@day.com> wrote:
> Hi,
>
> I'm not sure if this will help more than it will complicate things.
> Disadvantages:
>
> - Isn't almost every class at in o.a.j.core at least somewhat session related?
>
> - If you move classes to other packages, you will have to make many
> method public.
i share tomas's concerns.
>
> Instead of moving session related classes to a separate package, what
> about moving unrelated classes to different packages? For example
> TestContentLoader (>test), RepositoryCopier (>utilities),
> SearchManager (>search), NodeTypeInstanceHandler (>nodetype),
> RepositoryChecker (>persistence), UserPerWorkspaceSecurityManager
> (>security), DefaultSecurityManager (>security), ItemValidator
> (>nodetype).
+1
cheers
stefan
>
> Regards,
> Thomas
>
>
> On Mon, May 17, 2010 at 10:43 AM, Jukka Zitting <ju...@gmail.com> wrote:
>> Hi,
>>
>> As a part of my work on JCR-890, I'm planning to move most of the
>> session-related classes from o.a.j.core to a new o.a.j.core.session
>> package. This will make it easier to review and control related
>> dependencies and code paths, and to ultimately guard them against
>> access from concurrent threads.
>>
>> As the first step I'm simply moving the relevant classes and making
>> the minor dependency changes where needed, so the functional risk
>> should be low. However, the moves will likely invalidate many other
>> pending jackrabbit-core changes, so please let me know if you have
>> pending changes that I should wait for before I move these classes.
>> Unless there's a need to wait, I'm planning to commit the changes in
>> the afternoon today.
>>
>> BR,
>>
>> Jukka Zitting
>>
>
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Guo Du <mr...@duguo.com>.
On Mon, May 17, 2010 at 10:16 AM, Jukka Zitting <ju...@gmail.com> wrote:
> These unrelated classes are mostly things like RepositoryImpl,
> TransientRepository, RepositoryCopier, etc. to which many external
> codebases are linking, so we can't move them.
All class inside core has possible reference for user code base. It's
a good discussion which package those classes should be in ideally,
but do it for next major release such as jr3.
-Guo
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Thomas Müller <th...@day.com>.
Hi,
As far as I understand, you want to move the classes so we can add
checkstyle / PMD constraints, and more easily ensure every method call
from an external class is synchronized. I think that's fine.
Having the 'proxy' classes sounds like a solution for the backward
compatibility concerns (not the "perfect" solution, but a good
solution for Jackrabbit 2). For Jackrabbit 3 I hope people will not
directly cast to implementation classes any longer.
Regards,
Thomas
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, May 17, 2010 at 11:34 AM, Thomas Müller <th...@day.com> wrote:
>> These unrelated classes are mostly things like RepositoryImpl,
>> TransientRepository, RepositoryCopier, etc. to which many external
>> codebases are linking, so we can't move them.
>
> SessionImpl is used in my applications as well.
I was thinking of leaving simple compatibility wrappers like
@deprecated
public class SessionImpl extends o.a.j.core.session.SessionImpl { ... }
in o.a.j.core for at least SessionImpl, NodeImpl and PropertyImpl.
This should cover the needs of applications that use casts to these
classes.
The alternative approach to moving these classes to a separate package
is to move most of their internals to separate classes like a
SessionState where we can better control concurrent access to
per-session resources. However, I would prefer to start by moving the
classes since that is a much lower-risk operation that already helps
quite a bit.
BR,
Jukka Zitting
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Thomas Müller <th...@day.com>.
Hi,
> These unrelated classes are mostly things like RepositoryImpl,
> TransientRepository, RepositoryCopier, etc. to which many external
> codebases are linking, so we can't move them.
SessionImpl is used in my applications as well.
> RepositoryImpl,
> TransientRepository
I don't think those should be or need to be moved.
Regards,
Thomas
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Mon, May 17, 2010 at 11:00 AM, Thomas Müller <th...@day.com> wrote:
> - If you move classes to other packages, you will have to make many
> method public.
My ultimate goal is to track down all these cases and refactor them
away so we can better control all the code paths to and from a single
session. By moving the session-related classes to their own package
I'm able to use tools like jdepend to enforce strict access patterns.
This is by no means a trivial refactoring, but I believe it's
necessary since the complexity of the current o.a.j.core package makes
implementing JCR-890 close to impossible.
> Instead of moving session related classes to a separate package, what
> about moving unrelated classes to different packages?
These unrelated classes are mostly things like RepositoryImpl,
TransientRepository, RepositoryCopier, etc. to which many external
codebases are linking, so we can't move them.
BR,
Jukka Zitting
Re: FYI: Moving session-related classes to o.a.j.core.session
Posted by Thomas Müller <th...@day.com>.
Hi,
I'm not sure if this will help more than it will complicate things.
Disadvantages:
- Isn't almost every class at in o.a.j.core at least somewhat session related?
- If you move classes to other packages, you will have to make many
method public.
Instead of moving session related classes to a separate package, what
about moving unrelated classes to different packages? For example
TestContentLoader (>test), RepositoryCopier (>utilities),
SearchManager (>search), NodeTypeInstanceHandler (>nodetype),
RepositoryChecker (>persistence), UserPerWorkspaceSecurityManager
(>security), DefaultSecurityManager (>security), ItemValidator
(>nodetype).
Regards,
Thomas
On Mon, May 17, 2010 at 10:43 AM, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> As a part of my work on JCR-890, I'm planning to move most of the
> session-related classes from o.a.j.core to a new o.a.j.core.session
> package. This will make it easier to review and control related
> dependencies and code paths, and to ultimately guard them against
> access from concurrent threads.
>
> As the first step I'm simply moving the relevant classes and making
> the minor dependency changes where needed, so the functional risk
> should be low. However, the moves will likely invalidate many other
> pending jackrabbit-core changes, so please let me know if you have
> pending changes that I should wait for before I move these classes.
> Unless there's a need to wait, I'm planning to commit the changes in
> the afternoon today.
>
> BR,
>
> Jukka Zitting
>