Hi,
Interesting approach. This is quite similar to what I have done in AbstractNodeTypeManager. There I provide the abstract method getTypes() for accessing that specific node in the tree which carries the node types. This let's you instantiate a (read only) node type manager from a NodeState (no need for a ContentSession).
I think we should unify the approach and extend it to other areas where applicable. The advantages are that there is no code duplication for functionality required in oak-jcr and oak-core and that these classes can be instantiated ad hoc without the need for passing instances of these around.
Michael On 18.9.12 10:04, [email protected] wrote:
Author: jukka Date: Tue Sep 18 09:04:05 2012 New Revision: 1387062 URL: http://svn.apache.org/viewvc?rev=1387062&view=rev Log: OAK-306: Limit session refresh on namespace registry use Change getReadRoot() to getReadTree() so we can more easily use things like the ReadOnlyTree class if/when needed. Better documentation of this solution. Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImpl.java jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImplTest.java jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImpl.java?rev=1387062&r1=1387061&r2=1387062&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImpl.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImpl.java Tue Sep 18 09:04:05 2012 @@ -37,9 +37,29 @@ import org.apache.jackrabbit.oak.core.De public abstract class NamespaceRegistryImpl implements NamespaceRegistry, NamespaceConstants { - abstract protected Root getReadRoot(); + /** + * Called by the {@link NamespaceRegistry} implementation methods + * to acquire a root {@link Tree} instance from which to read the + * namespace mappings (under <code>jcr:system/rep:namespaces</code>). + * + * @return root {@link Tree} for reading the namespace mappings + */ + abstract protected Tree getReadTree(); - abstract protected Root getWriteRoot(); + /** + * Called by the {@link #registerNamespace(String, String)} and + * {@link #unregisterNamespace(String)} methods to acquire a fresh + * {@link Root} instance that can be used to persist the requested + * namespace changes (and nothing else). + * <p> + * The default implementation of this method throws an + * {@link UnsupportedOperationException}. + * + * @return fresh {@link Root} instance + */ + protected Root getWriteRoot() { + throw new UnsupportedOperationException(); + } /** * Called by the {@link NamespaceRegistry} implementation methods to @@ -59,7 +79,8 @@ public abstract class NamespaceRegistryI throws RepositoryException { try { Root root = getWriteRoot(); - Tree namespaces = getOrCreate(root, JcrConstants.JCR_SYSTEM, REP_NAMESPACES); + Tree namespaces = + getOrCreate(root, JcrConstants.JCR_SYSTEM, REP_NAMESPACES); namespaces.setProperty(prefix, new StringValue(uri)); root.commit(DefaultConflictHandler.OURS); refresh(); @@ -112,7 +133,7 @@ public abstract class NamespaceRegistryI @Nonnull public String[] getPrefixes() throws RepositoryException { try { - Tree root = getReadRoot().getTree("/"); + Tree root = getReadTree(); Map<String, String> map = Namespaces.getNamespaceMap(root); String[] prefixes = map.keySet().toArray(new String[map.size()]); Arrays.sort(prefixes); @@ -127,7 +148,7 @@ public abstract class NamespaceRegistryI @Nonnull public String[] getURIs() throws RepositoryException { try { - Tree root = getReadRoot().getTree("/"); + Tree root = getReadTree(); Map<String, String> map = Namespaces.getNamespaceMap(root); String[] uris = map.values().toArray(new String[map.size()]); Arrays.sort(uris); @@ -142,7 +163,7 @@ public abstract class NamespaceRegistryI @Nonnull public String getURI(String prefix) throws RepositoryException { try { - Tree root = getReadRoot().getTree("/"); + Tree root = getReadTree(); Map<String, String> map = Namespaces.getNamespaceMap(root); String uri = map.get(prefix); if (uri == null) { @@ -161,7 +182,7 @@ public abstract class NamespaceRegistryI @Nonnull public String getPrefix(String uri) throws RepositoryException { try { - Tree root = getReadRoot().getTree("/"); + Tree root = getReadTree(); Map<String, String> map = Namespaces.getNamespaceMap(root); for (Map.Entry<String, String> entry : map.entrySet()) { if (entry.getValue().equals(uri)) { Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImplTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImplTest.java?rev=1387062&r1=1387061&r2=1387062&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImplTest.java (original) +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/NamespaceRegistryImplTest.java Tue Sep 18 09:04:05 2012 @@ -22,6 +22,7 @@ import org.apache.jackrabbit.oak.Abstrac import org.apache.jackrabbit.oak.api.ContentRepository; import org.apache.jackrabbit.oak.api.ContentSession; import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.api.Tree; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -38,8 +39,8 @@ public class NamespaceRegistryImplTest e final ContentSession session = createAdminSession(); NamespaceRegistry r = new NamespaceRegistryImpl() { @Override - protected Root getReadRoot() { - return session.getLatestRoot(); + protected Tree getReadTree() { + return session.getLatestRoot().getTree("/"); } @Override protected Root getWriteRoot() { Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java?rev=1387062&r1=1387061&r2=1387062&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java (original) +++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java Tue Sep 18 09:04:05 2012 @@ -33,6 +33,7 @@ import javax.jcr.version.VersionManager; import org.apache.jackrabbit.api.JackrabbitWorkspace; import org.apache.jackrabbit.api.security.authorization.PrivilegeManager; import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.jcr.lock.LockManagerImpl; import org.apache.jackrabbit.oak.jcr.query.QueryManagerImpl; @@ -152,8 +153,8 @@ public class WorkspaceImpl implements Ja getSession().refresh(true); } @Override - protected Root getReadRoot() { - return sessionDelegate.getRoot(); + protected Tree getReadTree() { + return sessionDelegate.getRoot().getTree("/"); } @Override protected Root getWriteRoot() {
