On Dec 7, 2010, at 4:00 AM, Felix Meschberger wrote: > Hi, > > Am Montag, den 06.12.2010, 21:52 -0500 schrieb Justin Edelson: >> Hi, >> I'm attempting to upgrade Sling to use the current Jackrabbit 2.1.x release >> and ran into a fairly significant snag. >> >> In r950440 (as part of JCR-2573), >> org.apache.jackrabbit.core.security.AccessManager.canRead() was changed from >> a single argument (Path) method to a two-argument method (Path, ItemId). In >> o.a.s.jcr.jackrabbit.server.impl.security.PluggableDefaultAccessManager, we >> override this method (from o.a.j.core.security.DefaultAccessManager) and >> delegate to an implementation of the AccessManagerPlugin if available. > > Even though Jackrabbit probably considers this internal/private API, it > is IMHO not acceptable to have a breaking API change in a micro release > (am I understanding this correctly that this happened between 2.1.2 and > 2.1.3 ?) ... In fact it is even hard to accept in a minor release.
This happened between 2.1.1 and 2.1.2. > > This issue should be raised with the Jackrabbit project. > Yeah... > WDYT ? > >> >> The obviously easy thing to do is to modify AccessManagerPlugin to have a >> two-argument canRead() method, but this will both break any existing >> implementations of AccessManagerPlugin *and* will require that >> o.a.j.core.idbe exported by the jackrabbit-server bundle (there are >> probably other >> downsides to making this change - these are just the first two I can think >> of). On the other hand, I don't think it is quite tenable for us to be stuck >> at Jackrabbit 2.1.1 forever, so we may just need to bite this bullet, do a > > I am not a fan of exposing Jackrabbit Core packages. But maybe in this > case, it might work. > This is definitely the lesser problem. > But we must try to not break our own API .... This is what I am weary > of. > > So here is my proposal: > > * Get back at Jackrabbit to check for the incompatible API change > * Keep on supporting the old API (can we ?) Sure. The problem is that if canRead() is called with a null path and a non-null ItemId, an old AccessManagerPlugin won't be invoked. > * Add a new interface extending the old adding the new method > > Does this make sense? We'll see what jackrabbit-dev has to say... Justin > > Regards > Felix > >> major version change and roll with it. However, in this case, I'd be >> inclined to wait until Jackrabbit 2.2.0 is released. >> >> Changeset is here: http://codereview.appspot.com/3490041 >> >> WDYT? >> >> Justin >> >> P.S. Has anyone gotten reviews.apache.org to work? > >
