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 md...@apache.org on 2013/07/30 15:56:53 UTC

svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Author: mduerig
Date: Tue Jul 30 13:56:53 2013
New Revision: 1508445

URL: http://svn.apache.org/r1508445
Log:
OAK-803: Backwards compatibility of long-lived sessions
Preliminary support for JCR-3634

Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionContext.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java?rev=1508445&r1=1508444&r2=1508445&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/RepositoryImpl.java Tue Jul 30 13:56:53 2013
@@ -17,10 +17,13 @@
 package org.apache.jackrabbit.oak.jcr;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static java.util.Collections.singletonMap;
+import static java.util.concurrent.TimeUnit.SECONDS;
 
+import java.util.Collections;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.jcr.Credentials;
@@ -57,17 +60,12 @@ public class RepositoryImpl implements R
      *
      * @see SessionDelegate#SessionDelegate(ContentSession, long)
      */
-    public static final String REFRESH_INTERVAL = "refresh-interval";
-
-    /**
-     * Name of the session attribute value determining works space name.
-     */
-    public static final String WORKSPACE_NAME = "workspace-name";
+    public static final String REFRESH_INTERVAL = "oak.refresh-interval";
 
     /**
      * Default value for {@link #REFRESH_INTERVAL}.
      */
-    private static final long DEFAULT_REFRESH_INTERVAL = TimeUnit.SECONDS.toMillis(1);
+    private static final long DEFAULT_REFRESH_INTERVAL = SECONDS.toMillis(1);
 
     private final Descriptors descriptors = new Descriptors(new SimpleValueFactory());
     private final ContentRepository contentRepository;
@@ -140,11 +138,32 @@ public class RepositoryImpl implements R
     }
 
     // TODO make this method available through JackrabbitRepository. See OAK-803, JCR-3634
-    public Session login(Credentials credentials, Map<String, Object> attributes)
-            throws RepositoryException {
-        return login(credentials,
-                getString(attributes, WORKSPACE_NAME),
-                getLong(attributes, REFRESH_INTERVAL, DEFAULT_REFRESH_INTERVAL));
+    public Session login(@CheckForNull Credentials credentials, @CheckForNull String workspaceName,
+            @CheckForNull Map<String, Object> attributes) throws RepositoryException {
+        try {
+            if (attributes == null) {
+                attributes = Collections.emptyMap();
+            }
+            Long refreshInterval = getRefreshInterval(credentials);
+            if (refreshInterval == null) {
+                refreshInterval = getLong(attributes, REFRESH_INTERVAL);
+            }
+            if (refreshInterval == null) {
+                log.info("Setting " + REFRESH_INTERVAL + " to default value of '" +
+                        DEFAULT_REFRESH_INTERVAL + "' seconds.");
+                refreshInterval = DEFAULT_REFRESH_INTERVAL;
+            } else {
+                log.info("Setting " + REFRESH_INTERVAL + " to '" + refreshInterval + "' seconds.");
+            }
+
+            ContentSession contentSession = contentRepository.login(credentials, workspaceName);
+            SessionContext context = new SessionContext(this, whiteboard,
+                    singletonMap(REFRESH_INTERVAL, refreshInterval),
+                    new SessionDelegate(contentSession, refreshInterval));
+            return context.getSession();
+        } catch (LoginException e) {
+            throw new javax.jcr.LoginException(e.getMessage(), e);
+        }
     }
 
     /**
@@ -153,7 +172,7 @@ public class RepositoryImpl implements R
     @Override
     public Session login(@Nullable Credentials credentials, @Nullable String workspaceName)
             throws RepositoryException {
-        return login(credentials, workspaceName, getRefreshInterval(credentials));
+        return login(credentials, workspaceName, null);
     }
 
     /**
@@ -165,7 +184,7 @@ public class RepositoryImpl implements R
      */
     @Override
     public Session login() throws RepositoryException {
-        return login(null, (String) null);
+        return login(null, null, null);
     }
 
     /**
@@ -178,7 +197,7 @@ public class RepositoryImpl implements R
      */
     @Override
     public Session login(Credentials credentials) throws RepositoryException {
-        return login(credentials, (String) null);
+        return login(credentials, null, null);
     }
 
     /**
@@ -191,7 +210,7 @@ public class RepositoryImpl implements R
      */
     @Override
     public Session login(String workspace) throws RepositoryException {
-        return login(null, workspace);
+        return login(null, workspace, null);
     }
 
     //------------------------------------------------------------< internal >---
@@ -206,34 +225,21 @@ public class RepositoryImpl implements R
 
     //------------------------------------------------------------< private >---
 
-    private Session login(
-            @Nullable Credentials credentials, @Nullable String workspaceName,
-            long refreshInterval) throws RepositoryException {
-        try {
-            ContentSession contentSession = contentRepository.login(credentials, workspaceName);
-            SessionContext context = new SessionContext(this, whiteboard,
-                    new SessionDelegate(contentSession, refreshInterval));
-            return context.getSession();
-        } catch (LoginException e) {
-            throw new javax.jcr.LoginException(e.getMessage(), e);
-        }
-    }
-
-    private static long getRefreshInterval(Credentials credentials) {
+    private static Long getRefreshInterval(Credentials credentials) {
         if (credentials instanceof SimpleCredentials) {
             Object value = ((SimpleCredentials) credentials).getAttribute(REFRESH_INTERVAL);
-            return toLong(value, DEFAULT_REFRESH_INTERVAL);
+            return toLong(value);
         } else if (credentials instanceof TokenCredentials) {
             String value = ((TokenCredentials) credentials).getAttribute(REFRESH_INTERVAL);
             if (value != null) {
                 return toLong(value);
             }
         }
-        return DEFAULT_REFRESH_INTERVAL;
+        return null;
     }
 
-    private static long getLong(Map<String, Object> attributes, String name, long defaultValue) {
-        return toLong(attributes.get(name), defaultValue);
+    private static Long getLong(Map<String, Object> attributes, String name) {
+        return toLong(attributes.get(name));
     }
 
     private static String getString(Map<String, Object> attributes, String name) {
@@ -245,26 +251,25 @@ public class RepositoryImpl implements R
         }
     }
 
-    private static long toLong(Object value, long defaultValue) {
+    private static Long toLong(Object value) {
         if (value instanceof Long) {
             return (Long) value;
         } else if (value instanceof Integer) {
-            return (Integer) value;
+            return ((Integer) value).longValue();
         } else if (value instanceof String) {
             return toLong((String) value);
         } else {
-            return defaultValue;
+            return null;
         }
     }
 
-    private static long toLong(String longValue) {
+    private static Long toLong(String longValue) {
         try {
-            return Long.parseLong(longValue);
+            return Long.valueOf(longValue);
         } catch (NumberFormatException e) {
             log.warn("Invalid value '" + longValue + "' for " + REFRESH_INTERVAL +
-                    ". Expected long. Defaulting to '" + DEFAULT_REFRESH_INTERVAL +
-                    "' seconds .", e);
-            return DEFAULT_REFRESH_INTERVAL;
+                    ". Expected long. ", e);
+            return null;
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionContext.java?rev=1508445&r1=1508444&r2=1508445&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionContext.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionContext.java Tue Jul 30 13:56:53 2013
@@ -16,9 +16,12 @@
  */
 package org.apache.jackrabbit.oak.jcr;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.jcr.PathNotFoundException;
@@ -65,8 +68,6 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
 import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 /**
  * Instances of this class are passed to all JCR implementation classes
  * (e.g. {@code SessionImpl}, {@code NodeImpl}, etc.) and provide access to
@@ -92,7 +93,7 @@ public class SessionContext implements N
 
     SessionContext(
             RepositoryImpl repository, Whiteboard whiteboard,
-            final SessionDelegate delegate) {
+            Map<String, Long> attributes, final SessionDelegate delegate) {
         this.repository = checkNotNull(repository);
         this.whiteboard = checkNotNull(whiteboard);
         this.delegate = checkNotNull(delegate);
@@ -113,7 +114,7 @@ public class SessionContext implements N
         this.valueFactory = new ValueFactoryImpl(
                 delegate.getRoot().getBlobFactory(), namePathMapper);
 
-        this.session = new SessionImpl(this);
+        this.session = new SessionImpl(this, attributes);
         this.workspace = new WorkspaceImpl(this);
     }
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java?rev=1508445&r1=1508444&r2=1508445&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java Tue Jul 30 13:56:53 2013
@@ -16,10 +16,16 @@
  */
 package org.apache.jackrabbit.oak.jcr;
 
+import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.security.AccessControlException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.jcr.AccessDeniedException;
@@ -41,6 +47,7 @@ import javax.jcr.nodetype.ConstraintViol
 import javax.jcr.retention.RetentionManager;
 import javax.jcr.security.AccessControlManager;
 
+import com.google.common.collect.Sets;
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.api.security.user.UserManager;
@@ -65,8 +72,6 @@ import org.slf4j.LoggerFactory;
 import org.xml.sax.ContentHandler;
 import org.xml.sax.SAXException;
 
-import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
-
 /**
  * TODO document
  */
@@ -75,10 +80,12 @@ public class SessionImpl implements Jack
 
     private final SessionContext sessionContext;
     private final SessionDelegate sd;
+    private final Map<String, Long> attributes;
 
-    SessionImpl(SessionContext sessionContext) {
+    SessionImpl(SessionContext sessionContext, Map<String, Long> attributes) {
         this.sessionContext = sessionContext;
         this.sd = sessionContext.getSessionDelegate();
+        this.attributes = attributes;
     }
 
     static void checkIndexOnName(SessionContext sessionContext, String path) throws RepositoryException {
@@ -221,12 +228,15 @@ public class SessionImpl implements Jack
 
     @Override
     public String[] getAttributeNames() {
-        return sd.getAuthInfo().getAttributeNames();
+        Set<String> names = Sets.newHashSet(attributes.keySet());
+        Collections.addAll(names, sd.getAuthInfo().getAttributeNames());
+        return names.toArray(new String[names.size()]);
     }
 
     @Override
     public Object getAttribute(String name) {
-        return sd.getAuthInfo().getAttribute(name);
+        Object attribute = sd.getAuthInfo().getAttribute(name);
+        return attribute == null ? attributes.get(name) : attribute;
     }
 
     @Override



Re: svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Posted by Michael Dürig <md...@apache.org>.

On 30.7.13 16:40, Jukka Zitting wrote:
> There's always Session.getAttribute() (which BTW would be useful
> information to expose also through JMX)

Right. Let's do that.

Michael

Re: svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Tue, Jul 30, 2013 at 5:34 PM, Michael Dürig <md...@apache.org> wrote:
> On 30.7.13 16:24, Jukka Zitting wrote:
>> The user already knows the settings as they're the one providing the
>> information (or using the default).
>
> He knows what he passed in. That might be different from what is being
> applied. See the discussion re. precedence.

There's always Session.getAttribute() (which BTW would be useful
information to expose also through JMX) to check what the actual value
is if the client doesn't control all the arguments passed to login().
The benefit/cost of logging that information seems pretty low to me.

BR,

Jukka Zitting

Re: svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Posted by Michael Dürig <md...@apache.org>.

On 30.7.13 16:24, Jukka Zitting wrote:
> Hi,
>
> On Tue, Jul 30, 2013 at 5:15 PM, Michael Dürig <mi...@gmail.com> wrote:
>> On 30.7.13 16:09, Jukka Zitting wrote:
>>> On Tue, Jul 30, 2013 at 4:56 PM,  <md...@apache.org> wrote:
>>>> +                log.info(....);
>>>
>>> These should become log.debug(...) as soon as the code stabilizes.
>>
>> I don't agree. As long as this is user configurable we should make that
>> information available in the logs so the user can see the actual settings.
>
> The user already knows the settings as they're the one providing the
> information (or using the default).

He knows what he passed in. That might be different from what is being 
applied. See the discussion re. precedence.

> If we leave this at info, then we'll have an info log message
> associated with each new session, which during peak loads could mean
> hundreds or thousands of entries per second. IMO that's not the
> coarse-grained level referred to in [1].

There's a bunch of other problems here. I'll remove the logging entirely 
for the time being.

Michael

>
> [1] http://slf4j.org/apidocs/org/apache/log4j/Level.html#INFO
>
> BR,
>
> Jukka Zitting
>

Re: svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Tue, Jul 30, 2013 at 5:15 PM, Michael Dürig <mi...@gmail.com> wrote:
> On 30.7.13 16:09, Jukka Zitting wrote:
>> On Tue, Jul 30, 2013 at 4:56 PM,  <md...@apache.org> wrote:
>>> +                log.info(....);
>>
>> These should become log.debug(...) as soon as the code stabilizes.
>
> I don't agree. As long as this is user configurable we should make that
> information available in the logs so the user can see the actual settings.

The user already knows the settings as they're the one providing the
information (or using the default).

If we leave this at info, then we'll have an info log message
associated with each new session, which during peak loads could mean
hundreds or thousands of entries per second. IMO that's not the
coarse-grained level referred to in [1].

[1] http://slf4j.org/apidocs/org/apache/log4j/Level.html#INFO

BR,

Jukka Zitting

Re: svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Posted by Michael Dürig <mi...@gmail.com>.

On 30.7.13 16:09, Jukka Zitting wrote:
> Hi,
>
> On Tue, Jul 30, 2013 at 4:56 PM,  <md...@apache.org> wrote:
>> +                log.info(....);
>
> These should become log.debug(...) as soon as the code stabilizes.

I don't agree. As long as this is user configurable we should make that 
information available in the logs so the user can see the actual 
settings. Debug is fraught up with internal stuff that users shouldn't 
be seeing.

Michael

Re: svn commit: r1508445 - in /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr: RepositoryImpl.java SessionContext.java SessionImpl.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Tue, Jul 30, 2013 at 4:56 PM,  <md...@apache.org> wrote:
> +                log.info(....);

These should become log.debug(...) as soon as the code stabilizes.

BR,

Jukka Zitting