You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2012/09/18 11:04:05 UTC
svn commit: r1387062 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
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() {
Re: svn commit: r1387062 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
Posted by Michael Dürig <md...@apache.org>.
On 18.9.12 10:44, Jukka Zitting wrote:
> Hi,
>
> On Tue, Sep 18, 2012 at 11:40 AM, Michael Dürig <md...@apache.org> wrote:
>> 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.
>
> +1 This approach already opened up a chance to simplify some of the
> name validation code in .oak.plugins.name (no need for the extra
> Namespaces utility class), on which I'm just now working.
>
IdentifierManager is another candidate: resolving references is also
needed in core for checking reference value constraints.
Michael
> BR,
>
> Jukka Zitting
>
Re: svn commit: r1387062 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
Posted by Michael Dürig <md...@apache.org>.
On 18.9.12 10:44, Jukka Zitting wrote:
> Hi,
>
> On Tue, Sep 18, 2012 at 11:40 AM, Michael Dürig <md...@apache.org> wrote:
>> 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.
>
> +1 This approach already opened up a chance to simplify some of the
> name validation code in .oak.plugins.name (no need for the extra
> Namespaces utility class), on which I'm just now working.
Taking this further, the alignment could probably also be extended to
AbstractNameMapper, which follows a similar pattern. Currently there is
only one implementation, which is 'session based'. Providing an
implementation based on a read tree, NameMapper and NamePathMapper
instances could also be used from within oak-core.
Even further this decouples ValueImple, ValueFactoryImpl and related
from the session. So we could move these also to oak-core. Now while we
are at it, is there then still a reason for having a CoreValue
implementations and ValueImpl? Why not just make CoreValue extend
javax.jcr.Value?
BTW: javax.jcr.Value is indeed required in oak-core for checking value
constraints on node types. Currently this is solved by introducing (yet
another) value implementation: JcrValue implements javax.jcr.Value which
we could get rid off at the same time also.
Thoughts?
Michael
Re: svn commit: r1387062 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Tue, Sep 18, 2012 at 11:40 AM, Michael Dürig <md...@apache.org> wrote:
> 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.
+1 This approach already opened up a chance to simplify some of the
name validation code in .oak.plugins.name (no need for the extra
Namespaces utility class), on which I'm just now working.
BR,
Jukka Zitting
Re: svn commit: r1387062 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/name/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/name/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/
Posted by Michael Dürig <md...@apache.org>.
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, jukka@apache.org 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() {
>
>