tbouron commented on a change in pull request #1032: fix session sharing and simplify logout URL: https://github.com/apache/brooklyn-server/pull/1032#discussion_r252214010
########## File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/MultiSessionAttributeAdapter.java ########## @@ -0,0 +1,477 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.brooklyn.rest.util; + +import java.lang.reflect.Field; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; + +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.text.Strings; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.server.session.Session; +import org.eclipse.jetty.server.session.SessionHandler; +import org.osgi.framework.BundleContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Convenience to assist working with multiple sessions, ensuring requests in different bundles can + * get a consistent shared view of data. + * <p> + * Any processing that wants to {@link #getAttribute(String)}, {@link #setAttribute(String, Object)} or {@link #removeAttribute(String)} + * in a way that all Brooklyn bundles will see (e.g. authentication, etc) should use the methods in this class, + * as opposed to calling {@link HttpServletRequest#getSession()} then {@link HttpSession#getAttribute(String)}. + * <p> + * This class will follow heuristics to find a preferred shared session. The heuristics are as follows: + * <ul> + * <li>We look at the {@link Server} for a {@link #KEY_PREFERRED_SESSION_HANDLER_INSTANCE}, and + * if it has a session with {@link #KEY_IS_PREFERRED}, we use it + * <li>We look in all session handlers for a session marked as {@link #KEY_IS_PREFERRED} + * <li>If there is a {@link #KEY_PREFERRED_SESSION_HANDLER_INSTANCE} at the server, + * we create a session there (if needed, and if we can, ie we have the request) + * and mark it (or one already existing there) as {@link #KEY_IS_PREFERRED} + * <li>If there is no {@link #KEY_PREFERRED_SESSION_HANDLER_INSTANCE} at the server, + * we go through all bundles looking for the CXF one and we store that against the {@link Server} + * (this should only happen once, will log warnings if not found) + * <li>Finally we mark the originating session as the {@link #KEY_IS_PREFERRED} so that it + * is used in preference to others, including ones at the {@link #KEY_PREFERRED_SESSION_HANDLER_INSTANCE}, + * if we were invoked in a way where we couldn't find such a handler (ie no such bundle) and we couldn't create one there + * </ul> + * This class may have the occasional quirk if run in parallel but we can live with that, + * and the danger I think is confined to inconsistent sessions. + * It logs in that case and other fringe cases. + * <p> + * In future we may wish to make this more configurable, so bundles can specify a priority + * or a configuration property could specify the preferred bundles and/or context paths. + * But for now hard-coding CXF is fine. + * <p> + * Obviously code that bypasses this and uses the {@link HttpServletRequest#getSession()} + * will only read/write things in their session, and it won't be visible to requests from other bundles. + * <p> + * Previously we tried sharing sessions across bundles; you'll see in git history the code for that; + * however it was messy, and ended up being dodgy when we invalidate, and likely very dodgy if the + * system auto-invalidates (e.g. time- or memory-based expiry), as code there is quite fragile assuming + * sessions are one-to-one tied to their handlers. + */ +public class MultiSessionAttributeAdapter { + + private static final Logger log = LoggerFactory.getLogger(MultiSessionAttributeAdapter.class); + + private static final String KEY_PREFERRED_SESSION_HANDLER_INSTANCE = "org.apache.brooklyn.server.PreferredSessionHandlerInstance"; + private static final String KEY_IS_PREFERRED = "org.apache.brooklyn.server.IsPreferred"; + + private static final Object PREFERRED_SYMBOLIC_NAME = + "org.apache.cxf.cxf-rt-transports-http"; + //// our bundle here doesn't have a session handler; sessions to the REST API get the handler from CXF + //"org.apache.brooklyn.rest.rest-resources"; + + private final HttpSession preferredSession; + private final HttpSession localSession; + + private boolean silentlyAcceptLocalOnlyValues = false; + private boolean setLocalValuesAlso = false; + private boolean setAllKnownSessions = false; + + private static final Factory FACTORY = new Factory(); + + protected MultiSessionAttributeAdapter(HttpSession preferredSession, HttpSession localSession) { + this.preferredSession = preferredSession; + this.localSession = localSession; + } + + public static MultiSessionAttributeAdapter of(HttpServletRequest r) { + return of(r, true); + } + /** May return null iff create is false */ + public static MultiSessionAttributeAdapter of(HttpServletRequest r, boolean create) { + HttpSession session = r.getSession(create); + if (session==null) return null; + return new MultiSessionAttributeAdapter(FACTORY.findPreferredSession(r), session); + } + + /** Where the request isn't available, and the preferred session is expected to exist. + * Note we cannot create a new session at the preferred handler without a request. */ + public static MultiSessionAttributeAdapter of(HttpSession session) { + return new MultiSessionAttributeAdapter(FACTORY.findPreferredSession(session, null), session); + } + + + protected static class Factory { + + private HttpSession findPreferredSession(HttpServletRequest r) { + if (r.getSession(false)==null) { + log.warn("Creating session", new Exception("source of created session")); + r.getSession(); + } + return findPreferredSession(r.getSession(), r); + } + + private HttpSession findPreferredSession(HttpSession localSession, HttpServletRequest optionalRequest) { + if (localSession instanceof Session) { + SessionHandler preferredHandler = getPreferredJettyHandler((Session)localSession, true, true); + HttpSession preferredSession = preferredHandler==null ? null : preferredHandler.getHttpSession(localSession.getId()); + if (log.isTraceEnabled()) { + log.trace("Preferred session for "+info(optionalRequest, localSession)+": "+ + (preferredSession!=null ? info(preferredSession) : "none, willl make new session in "+info(preferredHandler))); + } + if (preferredSession!=null) { + return preferredSession; + } + if (preferredHandler!=null) { + if (optionalRequest!=null) { + HttpSession result = preferredHandler.newHttpSession(optionalRequest); + if (log.isTraceEnabled()) { + log.trace("Creating new session "+info(result)+" to be preferred for " + info(optionalRequest, localSession)); + } + return result; + } + // the server has a preferred handler, but no session yet; fall back to marking on the session + log.warn("No request so cannot create preferred session at preferred handler "+info(preferredHandler)+" for "+info(optionalRequest, localSession)+"; will exceptionally mark the calling session as the preferred one"); + markSessionAsPreferred(localSession, " (request came in for "+info(optionalRequest, localSession)+")"); + return localSession; + } else { + // shouldn't come here; at minimum it should have returned the local session's handler + log.warn("Unexpected failure to find a handler for "+info(optionalRequest, localSession)); + } + } else { + log.warn("Unsupported session impl in "+info(optionalRequest, localSession)); + } + return localSession; + } + + private SessionHandler getPreferredJettyHandler(Session localSession, boolean allowHandlerThatDoesntHaveSession, boolean markAndReturnThisIfNoneFound) { + SessionHandler localHandler = ((Session)localSession).getSessionHandler(); + Server server = localHandler.getServer(); + // NB: this can also be useful: ((DefaultSessionIdManager)localHandler.getSessionIdManager()) + + if (server!=null) { + Session sessionAtServerGlobalPreferredHandler = null; + + // does the server have a globally preferred handler + SessionHandler preferredServerGlobalSessionHandler = getServerGlobalPreferredHandler(server); + if (preferredServerGlobalSessionHandler!=null) { + sessionAtServerGlobalPreferredHandler = preferredServerGlobalSessionHandler.getSession(localSession.getId()); + if (sessionAtServerGlobalPreferredHandler!=null && Boolean.TRUE.equals( sessionAtServerGlobalPreferredHandler.getAttribute(KEY_IS_PREFERRED)) ) { + return preferredServerGlobalSessionHandler; + } + } + + Handler[] handlers = server.getChildHandlersByClass(SessionHandler.class); + + // if there is a session marked, use it, unless the server has a preferred session handler and it has an equivalent session + // this way if a session is marked (from use in a context where we don't have a web request) it will be used + SessionHandler preferredHandlerForMarkedSession = findPeerSessionMarkedPreferred(localSession.getId(), handlers); + if (preferredHandlerForMarkedSession!=null) return preferredHandlerForMarkedSession; + + // nothing marked as preferred; if server global handler has a session, mark it as preferred + // this way it will get found quickly on subsequent requests + if (sessionAtServerGlobalPreferredHandler!=null) { + sessionAtServerGlobalPreferredHandler.setAttribute(KEY_IS_PREFERRED, true); + return preferredServerGlobalSessionHandler; + } + + if (allowHandlerThatDoesntHaveSession && preferredServerGlobalSessionHandler!=null) { + return preferredServerGlobalSessionHandler; + } + if (preferredServerGlobalSessionHandler==null) { + preferredServerGlobalSessionHandler = findPreferredBundleHandler(localSession, server, handlers); + if (preferredServerGlobalSessionHandler!=null) { + // recurse + return getPreferredJettyHandler(localSession, allowHandlerThatDoesntHaveSession, markAndReturnThisIfNoneFound); + } + } + + if (markAndReturnThisIfNoneFound) { + // nothing detected as preferred ... let's mark this session as the preferred one + markSessionAsPreferred(localSession, " (this is the handler that the request came in on)"); + return localHandler; + } + + } else { + log.warn("Could not find server for "+info(localSession)); + } + return null; + } + + protected void markSessionAsPreferred(HttpSession localSession, String msg) { + if (log.isTraceEnabled()) { + log.trace("Recording on "+info(localSession)+" that it is the preferred session"+msg); + } + localSession.setAttribute(KEY_IS_PREFERRED, true); + } + + protected SessionHandler findPreferredBundleHandler(Session localSession, Server server, Handler[] handlers) { + if (PREFERRED_SYMBOLIC_NAME==null) return null; + + SessionHandler preferredHandler = null; + + if (handlers != null) { + for (Handler handler: handlers) { + SessionHandler sh = (SessionHandler) handler; + ContextHandler.Context ctx = getContext(sh); Review comment: I'm a bit concerned performance wise about this call, as `getContext(sh)` uses refection. This is done within a loop of handler which can be potentially big. If I read this code correctly, this will be executed only during the session creation, if there is no other sessions marked as preferred, so it is somewhat mitigated. But still, I'm wondering if it could be a bottleneck ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
