From cfd994548ea802df0b980ebca8df20daebbc9383 Mon Sep 17 00:00:00 2001 From: Colby Ranger Date: Wed, 9 May 2012 10:39:48 -0700 Subject: [PATCH] Update the ThreadLocal based scopes to use RequestContext. Bound the ThreadLocalRequestContext module, which makes the CurrentUser available in the Global module. Removed any binding in the scopes that provided the CurrentUser. Updated all of the scopes to propagate the RequestContext. The PerThreadRequestScope.Propagator allows scoping callables to enter a request context. Change-Id: Idf682ed1d7485cf8c9cdd22cd89bfe1ad5296880 --- .../httpd/HttpIdentifiedUserProvider.java | 41 -------- ...rProvider.java => HttpRequestContext.java} | 7 +- ...pFilter.java => RequestContextFilter.java} | 32 +++++- .../com/google/gerrit/httpd/WebModule.java | 12 +-- .../java/com/google/gerrit/pgm/Daemon.java | 2 + .../server/config/GerritGlobalModule.java | 6 ++ .../server/config/GerritRequestModule.java | 1 - .../gerrit/server/git/ChangeMergeQueue.java | 47 +++++---- .../server/git/PerThreadRequestScope.java | 98 +++++++++++-------- .../util/GuiceRequestScopePropagator.java | 9 +- .../server/util/RequestScopePropagator.java | 64 ++++++++---- .../ThreadLocalRequestScopePropagator.java | 4 +- .../gerrit/sshd/CommandFactoryProvider.java | 2 +- .../gerrit/sshd/DatabasePubKeyAuth.java | 4 +- .../java/com/google/gerrit/sshd/NoShell.java | 2 +- .../gerrit/sshd/SshCurrentUserProvider.java | 42 -------- .../sshd/SshIdentifiedUserProvider.java | 47 --------- .../com/google/gerrit/sshd/SshModule.java | 5 - .../java/com/google/gerrit/sshd/SshScope.java | 54 ++++++++-- .../gerrit/httpd/WebAppInitializer.java | 1 + 20 files changed, 222 insertions(+), 258 deletions(-) delete mode 100644 gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpIdentifiedUserProvider.java rename gerrit-httpd/src/main/java/com/google/gerrit/httpd/{HttpCurrentUserProvider.java => HttpRequestContext.java} (82%) rename gerrit-httpd/src/main/java/com/google/gerrit/httpd/{RequestCleanupFilter.java => RequestContextFilter.java} (61%) delete mode 100644 gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCurrentUserProvider.java delete mode 100644 gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshIdentifiedUserProvider.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpIdentifiedUserProvider.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpIdentifiedUserProvider.java deleted file mode 100644 index 6c420a5df6..0000000000 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpIdentifiedUserProvider.java +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (C) 2009 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.httpd; - -import com.google.gerrit.common.errors.NotSignedInException; -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; -import com.google.inject.Inject; -import com.google.inject.Provider; -import com.google.inject.ProvisionException; - -class HttpIdentifiedUserProvider implements Provider { - private final Provider currUserProvider; - - @Inject - HttpIdentifiedUserProvider(Provider currUserProvider) { - this.currUserProvider = currUserProvider; - } - - @Override - public IdentifiedUser get() { - CurrentUser user = currUserProvider.get(); - if (user instanceof IdentifiedUser) { - return (IdentifiedUser) user; - } - throw new ProvisionException(NotSignedInException.MESSAGE, - new NotSignedInException()); - } -} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpCurrentUserProvider.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpRequestContext.java similarity index 82% rename from gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpCurrentUserProvider.java rename to gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpRequestContext.java index c87a14306b..4c88240b03 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpCurrentUserProvider.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/HttpRequestContext.java @@ -15,19 +15,20 @@ package com.google.gerrit.httpd; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.util.RequestContext; import com.google.inject.Inject; import com.google.inject.Provider; -class HttpCurrentUserProvider implements Provider { +class HttpRequestContext implements RequestContext { private final WebSession session; @Inject - HttpCurrentUserProvider(final WebSession session) { + HttpRequestContext(final WebSession session) { this.session = session; } @Override - public CurrentUser get() { + public CurrentUser getCurrentUser() { return session.getCurrentUser(); } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestCleanupFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestContextFilter.java similarity index 61% rename from gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestCleanupFilter.java rename to gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestContextFilter.java index 0e6a567598..b46505f0e1 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestCleanupFilter.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestContextFilter.java @@ -15,9 +15,13 @@ package com.google.gerrit.httpd; import com.google.gerrit.server.RequestCleanup; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.inject.Inject; +import com.google.inject.Module; import com.google.inject.Provider; import com.google.inject.Singleton; +import com.google.inject.servlet.ServletModule; import java.io.IOException; @@ -30,12 +34,27 @@ import javax.servlet.ServletResponse; /** Executes any pending {@link RequestCleanup} at the end of a request. */ @Singleton -class RequestCleanupFilter implements Filter { +public class RequestContextFilter implements Filter { + public static Module module() { + return new ServletModule() { + @Override + protected void configureServlets() { + filter("/*").through(RequestContextFilter.class); + } + }; + } + private final Provider cleanup; + private final Provider requestContext; + private final ThreadLocalRequestContext local; @Inject - RequestCleanupFilter(final Provider r) { + RequestContextFilter(final Provider r, + final Provider c, + final ThreadLocalRequestContext l) { cleanup = r; + requestContext = c; + local = l; } @Override @@ -50,10 +69,15 @@ class RequestCleanupFilter implements Filter { public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { + RequestContext old = local.setContext(requestContext.get()); try { - chain.doFilter(request, response); + try { + chain.doFilter(request, response); + } finally { + cleanup.get().run(); + } } finally { - cleanup.get().run(); + local.setContext(old); } } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java index d198b93032..852caae1d3 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebModule.java @@ -25,8 +25,6 @@ import com.google.gerrit.httpd.auth.ldap.LdapAuthModule; import com.google.gerrit.httpd.gitweb.GitWebModule; import com.google.gerrit.httpd.rpc.UiRpcModule; import com.google.gerrit.lifecycle.LifecycleModule; -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.RemotePeer; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.ChangeUserName; @@ -78,13 +76,8 @@ public class WebModule extends FactoryModule { @Override protected void configure() { - install(new ServletModule() { - @Override - protected void configureServlets() { - filter("/*").through(RequestCleanupFilter.class); - } - }); bind(RequestScopePropagator.class).to(GuiceRequestScopePropagator.class); + bind(HttpRequestContext.class); if (wantSSL) { install(new RequireSslFilter.Module()); @@ -147,9 +140,6 @@ public class WebModule extends FactoryModule { bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider( HttpRemotePeerProvider.class).in(RequestScoped.class); - bind(CurrentUser.class).toProvider(HttpCurrentUserProvider.class); - bind(IdentifiedUser.class).toProvider(HttpIdentifiedUserProvider.class); - install(new LifecycleModule() { @Override protected void configure() { diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index 202d55bf61..c05948d073 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -21,6 +21,7 @@ import com.google.gerrit.ehcache.EhcachePoolImpl; import com.google.gerrit.httpd.CacheBasedWebSession; import com.google.gerrit.httpd.GitOverHttpModule; import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider; +import com.google.gerrit.httpd.RequestContextFilter; import com.google.gerrit.httpd.WebModule; import com.google.gerrit.httpd.WebSshGlueModule; import com.google.gerrit.httpd.auth.openid.OpenIdModule; @@ -271,6 +272,7 @@ public class Daemon extends SiteProgram { private Injector createWebInjector() { final List modules = new ArrayList(); + modules.add(RequestContextFilter.module()); modules.add(CacheBasedWebSession.module()); modules.add(HttpContactStoreConnection.module()); modules.add(sysInjector.getInstance(GitOverHttpModule.class)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index b46a819af6..28759206f2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -23,6 +23,7 @@ import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.rules.PrologModule; import com.google.gerrit.rules.RulesCache; +import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.FileTypeRegistry; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.InternalUser; @@ -64,8 +65,10 @@ import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.SectionSortCache; import com.google.gerrit.server.tools.ToolsCatalog; import com.google.gerrit.server.util.IdGenerator; +import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.server.workflow.FunctionState; import com.google.inject.Inject; +import com.google.inject.servlet.RequestScoped; import org.apache.velocity.runtime.RuntimeInstance; import org.eclipse.jgit.lib.Config; @@ -117,6 +120,7 @@ public class GerritGlobalModule extends FactoryModule { install(new AccessControlModule()); install(new GitModule()); install(new PrologModule()); + install(ThreadLocalRequestContext.module()); factory(AccountInfoCacheFactory.Factory.class); factory(CapabilityControl.Factory.class); @@ -154,5 +158,7 @@ public class GerritGlobalModule extends FactoryModule { bind(GitReferenceUpdated.class); DynamicSet.setOf(binder(), GitReferenceUpdatedListener.class); DynamicSet.setOf(binder(), NewProjectCreatedListener.class); + + bind(AnonymousUser.class); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index f581adc709..5d6ecc3f9d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java @@ -77,7 +77,6 @@ public class GerritRequestModule extends FactoryModule { bind(ListProjects.class); bind(ApprovalsUtil.class); - bind(AnonymousUser.class).in(RequestScoped.class); bind(PerRequestProjectControlCache.class).in(RequestScoped.class); bind(ChangeControl.Factory.class).in(SINGLETON); bind(GroupControl.Factory.class).in(SINGLETON); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java index 86e0740d18..6d1b1556a0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java @@ -20,16 +20,17 @@ import static java.util.concurrent.TimeUnit.SECONDS; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.RemotePeer; import com.google.gerrit.server.config.GerritRequestModule; import com.google.gerrit.server.ssh.SshInfo; +import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.RequestScopePropagator; import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.OutOfScopeException; import com.google.inject.Provider; +import com.google.inject.Provides; import com.google.inject.Singleton; import com.google.inject.servlet.RequestScoped; @@ -43,6 +44,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @Singleton @@ -57,6 +59,7 @@ public class ChangeMergeQueue implements MergeQueue { private final WorkQueue workQueue; private final Provider bgFactory; + private final PerThreadRequestScope.Scoper threadScoper; @Inject ChangeMergeQueue(final WorkQueue wq, Injector parent) { @@ -68,15 +71,9 @@ public class ChangeMergeQueue implements MergeQueue { bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST); bind(RequestScopePropagator.class) .to(PerThreadRequestScope.Propagator.class); + bind(PerThreadRequestScope.Propagator.class); install(new GerritRequestModule()); - bind(CurrentUser.class).to(IdentifiedUser.class); - bind(IdentifiedUser.class).toProvider(new Provider() { - @Override - public IdentifiedUser get() { - throw new OutOfScopeException("No user on merge thread"); - } - }); bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider( new Provider() { @Override @@ -91,8 +88,26 @@ public class ChangeMergeQueue implements MergeQueue { } }); } + + @Provides + public PerThreadRequestScope.Scoper provideScoper( + final PerThreadRequestScope.Propagator propagator) { + final RequestContext requestContext = new RequestContext() { + @Override + public CurrentUser getCurrentUser() { + throw new OutOfScopeException("No user on merge thread"); + } + }; + return new PerThreadRequestScope.Scoper() { + @Override + public Callable scope(Callable callable) { + return propagator.scope(requestContext, callable); + } + }; + } }); bgFactory = child.getProvider(MergeOp.Factory.class); + threadScoper = child.getInstance(PerThreadRequestScope.Scoper.class); } @Override @@ -186,19 +201,15 @@ public class ChangeMergeQueue implements MergeQueue { } } - private void mergeImpl(Branch.NameKey branch) { + private void mergeImpl(final Branch.NameKey branch) { try { - PerThreadRequestScope ctx = new PerThreadRequestScope(); - PerThreadRequestScope old = PerThreadRequestScope.set(ctx); - try { - try { + threadScoper.scope(new Callable(){ + @Override + public Void call() throws Exception { bgFactory.get().create(branch).merge(); - } finally { - ctx.cleanup.run(); + return null; } - } finally { - PerThreadRequestScope.set(old); - } + }).call(); } catch (Throwable e) { log.error("Merge attempt for " + branch + " failed", e); } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/PerThreadRequestScope.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/PerThreadRequestScope.java index 6b92ff0000..8aea73ad56 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/PerThreadRequestScope.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/PerThreadRequestScope.java @@ -14,50 +14,86 @@ package com.google.gerrit.server.git; -import com.google.gerrit.server.RequestCleanup; +import com.google.common.collect.Maps; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.server.util.ThreadLocalRequestScopePropagator; +import com.google.inject.Inject; import com.google.inject.Key; import com.google.inject.OutOfScopeException; import com.google.inject.Provider; import com.google.inject.Scope; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.Callable; public class PerThreadRequestScope { - static class Propagator - extends ThreadLocalRequestScopePropagator { - Propagator() { - super(REQUEST, current); + public interface Scoper { + Callable scope(Callable callable); + } + + private static class Context { + private final Map, Object> map; + + private Context() { + map = Maps.newHashMap(); } - @Override - protected PerThreadRequestScope continuingContext( - PerThreadRequestScope ctx) { - return new PerThreadRequestScope(); + private T get(Key key, Provider creator) { + @SuppressWarnings("unchecked") + T t = (T) map.get(key); + if (t == null) { + t = creator.get(); + map.put(key, t); + } + return t; } } - private static final ThreadLocal current = - new ThreadLocal(); + public static class Propagator extends ThreadLocalRequestScopePropagator { + @Inject + Propagator(ThreadLocalRequestContext local) { + super(REQUEST, current, local); + } - private static PerThreadRequestScope requireContext() { - final PerThreadRequestScope ctx = current.get(); + @Override + protected Context continuingContext(Context ctx) { + return new Context(); + } + + public Callable scope(RequestContext requestContext, Callable callable) { + final Context ctx = new Context(); + final Callable wrapped = context(requestContext, cleanup(callable)); + return new Callable() { + @Override + public T call() throws Exception { + Context old = current.get(); + current.set(ctx); + try { + return wrapped.call(); + } finally { + current.set(old); + } + } + }; + } + } + + private static final ThreadLocal current = new ThreadLocal(); + + private static Context requireContext() { + final Context ctx = current.get(); if (ctx == null) { throw new OutOfScopeException("Not in command/request"); } return ctx; } - public static PerThreadRequestScope set(PerThreadRequestScope ctx) { - PerThreadRequestScope old = current.get(); - current.set(ctx); - return old; - } - public static final Scope REQUEST = new Scope() { + @Override public Provider scope(final Key key, final Provider creator) { return new Provider() { + @Override public T get() { return requireContext().get(key, creator); } @@ -74,26 +110,4 @@ public class PerThreadRequestScope { return "PerThreadRequestScope.REQUEST"; } }; - - private static final Key RC_KEY = - Key.get(RequestCleanup.class); - - final RequestCleanup cleanup; - private final Map, Object> map; - - public PerThreadRequestScope() { - cleanup = new RequestCleanup(); - map = new HashMap, Object>(); - map.put(RC_KEY, cleanup); - } - - synchronized T get(Key key, Provider creator) { - @SuppressWarnings("unchecked") - T t = (T) map.get(key); - if (t == null) { - t = creator.get(); - map.put(key, t); - } - return t; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/GuiceRequestScopePropagator.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/GuiceRequestScopePropagator.java index 9befc7d190..cc3c2f7d1c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/util/GuiceRequestScopePropagator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/GuiceRequestScopePropagator.java @@ -38,17 +38,15 @@ public class GuiceRequestScopePropagator extends RequestScopePropagator { private final String url; private final SocketAddress peer; - private final CurrentUser user; @Inject GuiceRequestScopePropagator( @CanonicalWebUrl @Nullable Provider urlProvider, @RemotePeer Provider remotePeerProvider, - Provider currentUserProvider) { - super(ServletScopes.REQUEST); + ThreadLocalRequestContext local) { + super(ServletScopes.REQUEST, local); this.url = urlProvider != null ? urlProvider.get() : null; this.peer = remotePeerProvider.get(); - this.user = currentUserProvider.get(); } /** @@ -69,9 +67,6 @@ public class GuiceRequestScopePropagator extends RequestScopePropagator { Providers.of(peer)); seedMap.put(Key.get(SocketAddress.class, RemotePeer.class), peer); - seedMap.put(Key.get(typeOfProvider(CurrentUser.class)), Providers.of(user)); - seedMap.put(Key.get(CurrentUser.class), user); - return ServletScopes.continueRequest(callable, seedMap); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java index 3661aa25d7..84c61e946b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java @@ -42,9 +42,12 @@ import java.util.concurrent.Executors; public abstract class RequestScopePropagator { private final Scope scope; + private final ThreadLocalRequestContext local; - protected RequestScopePropagator(Scope scope) { + protected RequestScopePropagator(Scope scope, + ThreadLocalRequestContext local) { this.scope = scope; + this.local = local; } /** @@ -70,26 +73,8 @@ public abstract class RequestScopePropagator { * @return a new Callable which will execute in the current request scope. */ public final Callable wrap(final Callable callable) { - final Callable wrapped = wrapImpl(new Callable() { - @Override - public T call() throws Exception { - RequestCleanup cleanup = scope.scope( - Key.get(RequestCleanup.class), - new Provider() { - @Override - public RequestCleanup get() { - return new RequestCleanup(); - } - }).get(); - - try { - return callable.call(); - } finally { - cleanup.run(); - } - } - }); - + final Callable wrapped = + wrapImpl(context(local.getContext(), cleanup(callable))); return new Callable() { @Override public T call() throws Exception { @@ -178,4 +163,41 @@ public abstract class RequestScopePropagator { * @see #wrap(Callable) */ protected abstract Callable wrapImpl(final Callable callable); + + protected Callable context(final RequestContext context, + final Callable callable) { + return new Callable() { + @Override + public T call() throws Exception { + RequestContext old = local.setContext(context); + try { + return callable.call(); + } finally { + local.setContext(old); + } + } + }; + } + + protected Callable cleanup(final Callable callable) { + return new Callable() { + @Override + public T call() throws Exception { + RequestCleanup cleanup = scope.scope( + Key.get(RequestCleanup.class), + new Provider() { + @Override + public RequestCleanup get() { + return new RequestCleanup(); + } + }).get(); + + try { + return callable.call(); + } finally { + cleanup.run(); + } + } + }; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/ThreadLocalRequestScopePropagator.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/ThreadLocalRequestScopePropagator.java index 581ccc1483..e46524799b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/util/ThreadLocalRequestScopePropagator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/ThreadLocalRequestScopePropagator.java @@ -31,8 +31,8 @@ public abstract class ThreadLocalRequestScopePropagator private final ThreadLocal threadLocal; protected ThreadLocalRequestScopePropagator(Scope scope, - ThreadLocal threadLocal) { - super(scope); + ThreadLocal threadLocal, ThreadLocalRequestContext local) { + super(scope, local); this.threadLocal = threadLocal; } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java index 8eaa829294..be6659d12c 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java @@ -122,7 +122,7 @@ class CommandFactoryProvider implements Provider { public void setSession(final ServerSession session) { final SshSession s = session.getAttribute(SshSession.KEY); - this.ctx = new Context(s, commandLine); + this.ctx = sshScope.newContext(s, commandLine); } public void start(final Environment env) throws IOException { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java index 0b31a653e4..a69a2f1e38 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/DatabasePubKeyAuth.java @@ -173,7 +173,7 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator { // session, record a login event in the log and add // a close listener to record a logout event. // - Context ctx = new Context(sd, null); + Context ctx = sshScope.newContext(sd, null); Context old = sshScope.set(ctx); try { sshLog.onLogin(); @@ -185,7 +185,7 @@ class DatabasePubKeyAuth implements PublickeyAuthenticator { new IoFutureListener() { @Override public void operationComplete(IoFuture future) { - final Context ctx = new Context(sd, null); + final Context ctx = sshScope.newContext(sd, null); final Context old = sshScope.set(ctx); try { sshLog.onLogout(); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/NoShell.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/NoShell.java index 5ea2f6bdaa..d2fdf781d1 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/NoShell.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/NoShell.java @@ -89,7 +89,7 @@ class NoShell implements Factory { } public void setSession(final ServerSession session) { - this.context = new Context(session.getAttribute(SshSession.KEY), ""); + this.context = sshScope.newContext(session.getAttribute(SshSession.KEY), ""); } public void start(final Environment env) throws IOException { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCurrentUserProvider.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCurrentUserProvider.java deleted file mode 100644 index 5839cf17dc..0000000000 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCurrentUserProvider.java +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright (C) 2010 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.sshd; - -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; -import com.google.inject.Inject; -import com.google.inject.Provider; -import com.google.inject.Singleton; - -@Singleton -class SshCurrentUserProvider implements Provider { - private final Provider session; - private final Provider identifiedProvider; - - @Inject - SshCurrentUserProvider(Provider s, Provider p) { - session = s; - identifiedProvider = p; - } - - @Override - public CurrentUser get() { - final CurrentUser user = session.get().getCurrentUser(); - if (user instanceof IdentifiedUser) { - return identifiedProvider.get(); - } - return session.get().getCurrentUser(); - } -} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshIdentifiedUserProvider.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshIdentifiedUserProvider.java deleted file mode 100644 index 516b59cb43..0000000000 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshIdentifiedUserProvider.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (C) 2010 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.sshd; - -import com.google.gerrit.common.errors.NotSignedInException; -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.IdentifiedUser; -import com.google.inject.Inject; -import com.google.inject.Provider; -import com.google.inject.ProvisionException; -import com.google.inject.Singleton; - -@Singleton -class SshIdentifiedUserProvider implements Provider { - private final Provider session; - private final IdentifiedUser.RequestFactory factory; - - @Inject - SshIdentifiedUserProvider(Provider s, - IdentifiedUser.RequestFactory f) { - session = s; - factory = f; - } - - @Override - public IdentifiedUser get() { - final CurrentUser user = session.get().getCurrentUser(); - if (user instanceof IdentifiedUser) { - return factory.create(user.getAccessPath(), // - ((IdentifiedUser) user).getAccountId()); - } - throw new ProvisionException(NotSignedInException.MESSAGE, - new NotSignedInException()); - } -} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java index 72fe6b040c..bcb1d9fbb0 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java @@ -152,11 +152,6 @@ public class SshModule extends FactoryModule { bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider( SshRemotePeerProvider.class).in(SshScope.REQUEST); - bind(CurrentUser.class).toProvider(SshCurrentUserProvider.class).in( - SshScope.REQUEST); - bind(IdentifiedUser.class).toProvider(SshIdentifiedUserProvider.class).in( - SshScope.REQUEST); - bind(WorkQueue.Executor.class).annotatedWith(CommandExecutor.class) .toProvider(CommandExecutorProvider.class).in(SshScope.REQUEST); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java index 8cd501e6ca..d6f66ca1b6 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java @@ -14,8 +14,13 @@ package com.google.gerrit.sshd; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.RequestCleanup; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.server.util.ThreadLocalRequestScopePropagator; +import com.google.inject.Inject; import com.google.inject.Key; import com.google.inject.OutOfScopeException; import com.google.inject.Provider; @@ -26,10 +31,10 @@ import java.util.Map; /** Guice scopes for state during an SSH connection. */ class SshScope { - static class Context { - private static final Key RC_KEY = - Key.get(RequestCleanup.class); + private static final Key RC_KEY = + Key.get(RequestCleanup.class); + class Context implements RequestContext { private final RequestCleanup cleanup; private final SshSession session; private final String commandLine; @@ -56,10 +61,6 @@ class SshScope { finished = p.finished; } - Context(final SshSession s, final String c) { - this(s, c, System.currentTimeMillis()); - } - String getCommandLine() { return commandLine; } @@ -68,6 +69,16 @@ class SshScope { return session; } + @Override + public CurrentUser getCurrentUser() { + final CurrentUser user = session.getCurrentUser(); + if (user instanceof IdentifiedUser) { + return userFactory.create(user.getAccessPath(), // + ((IdentifiedUser) user).getAccountId()); + } + return user; + } + synchronized T get(Key key, Provider creator) { @SuppressWarnings("unchecked") T t = (T) map.get(key); @@ -100,15 +111,19 @@ class SshScope { } static class Propagator extends ThreadLocalRequestScopePropagator { - Propagator() { - super(REQUEST, current); + private final SshScope sshScope; + + @Inject + Propagator(SshScope sshScope, ThreadLocalRequestContext local) { + super(REQUEST, current, local); + this.sshScope = sshScope; } @Override protected Context continuingContext(Context ctx) { // The cleanup is not chained, since the RequestScopePropagator executors // the Context's cleanup when finished executing. - return new Context(ctx, ctx.getSession(), ctx.getCommandLine()); + return sshScope.newContinuingContext(ctx); } } @@ -123,9 +138,28 @@ class SshScope { return ctx; } + private final ThreadLocalRequestContext local; + private final IdentifiedUser.RequestFactory userFactory; + + @Inject + SshScope(ThreadLocalRequestContext local, + IdentifiedUser.RequestFactory userFactory) { + this.local = local; + this.userFactory = userFactory; + } + + Context newContext(SshSession session, String commandLine) { + return new Context(session, commandLine, System.currentTimeMillis()); + } + + private Context newContinuingContext(Context ctx) { + return new Context(ctx, ctx.getSession(), ctx.getCommandLine()); + } + Context set(Context ctx) { Context old = current.get(); current.set(ctx); + local.setContext(ctx); return old; } diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 0bb24cc835..58d61162e1 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -223,6 +223,7 @@ public class WebAppInitializer extends GuiceServletContextListener { private Injector createWebInjector() { final List modules = new ArrayList(); + modules.add(RequestContextFilter.module()); modules.add(sysInjector.getInstance(GitOverHttpModule.class)); modules.add(sshInjector.getInstance(WebModule.class)); modules.add(sshInjector.getInstance(WebSshGlueModule.class));