Make Dashboard directly immutable Dashboard still contains references to non-immutable structures, but the main internals no longer change.
diff --git a/src/com/google/enterprise/adaptor/Dashboard.java b/src/com/google/enterprise/adaptor/Dashboard.java index 80163dc..3064091 100644 --- a/src/com/google/enterprise/adaptor/Dashboard.java +++ b/src/com/google/enterprise/adaptor/Dashboard.java
@@ -33,55 +33,47 @@ private final Config config; private final Journal journal; - private HttpServerScope scope; - private CircularLogRpcMethod circularLogRpcMethod; - private final StatusMonitor monitor = new StatusMonitor(); + private final CircularLogRpcMethod circularLogRpcMethod + = new CircularLogRpcMethod(); private final GsaCommunicationHandler gsaCommHandler; private final SessionManager<HttpExchange> sessionManager; private final RpcHandler rpcHandler; - private final Adaptor adaptor; public Dashboard(Config config, GsaCommunicationHandler gsaCommHandler, Journal journal, SessionManager<HttpExchange> sessionManager, - SensitiveValueCodec secureValueCodec, Adaptor adaptor) { + SensitiveValueCodec secureValueCodec, Adaptor adaptor, + List<StatusSource> adaptorSources) { this.config = config; this.gsaCommHandler = gsaCommHandler; this.journal = journal; this.sessionManager = sessionManager; - this.adaptor = adaptor; + + List<StatusSource> sources = new LinkedList<StatusSource>(); + sources.add(new LastPushStatusSource(journal)); + sources.add(new RetrieverStatusSource(journal)); + sources.add(new GsaCrawlingStatusSource(journal)); + sources.addAll(adaptorSources); rpcHandler = new RpcHandler(sessionManager); rpcHandler.registerRpcMethod("startFeedPush", new StartFeedPushRpcMethod()); rpcHandler.registerRpcMethod("startIncrementalFeedPush", new StartIncrementalFeedPushRpcMethod()); - circularLogRpcMethod = new CircularLogRpcMethod(); rpcHandler.registerRpcMethod("getLog", circularLogRpcMethod); rpcHandler.registerRpcMethod("getConfig", new ConfigRpcMethod(config)); - rpcHandler.registerRpcMethod("getStatuses", new StatusRpcMethod(monitor)); + rpcHandler.registerRpcMethod("getStatuses", new StatusRpcMethod(sources)); rpcHandler.registerRpcMethod("checkForUpdatedConfig", new CheckForUpdatedConfigRpcMethod(gsaCommHandler)); rpcHandler.registerRpcMethod("encodeSensitiveValue", new EncodeSensitiveValueMethod(secureValueCodec)); - - monitor.addSource(new LastPushStatusSource(journal)); - monitor.addSource(new RetrieverStatusSource(journal)); - monitor.addSource(new GsaCrawlingStatusSource(journal)); + rpcHandler.registerRpcMethod("getStats", new StatRpcMethod(journal, adaptor, + gsaCommHandler.isAdaptorIncremental())); } /** Starts listening for connections to the dashboard. */ - public void start(HttpServer dashboardServer, String contextPrefix) - throws IOException { - rpcHandler.registerRpcMethod("getStats", new StatRpcMethod(journal, adaptor, - gsaCommHandler.isAdaptorIncremental())); - - this.scope = new HttpServerScope(dashboardServer, contextPrefix); - int dashboardPort = dashboardServer.getAddress().getPort(); - if (dashboardPort != config.getServerDashboardPort()) { - config.setValue("server.dashboardPort", "" + dashboardPort); - } + public void start(HttpServerScope scope) { boolean secure = config.isServerSecure(); HttpHandler dashboardHandler = new DashboardHandler(); - addFilters(scope.createContext("/dashboard", + HttpContext dashboardContext = addFilters(scope.createContext("/dashboard", createAdminSecurityHandler(dashboardHandler, config, sessionManager, secure))); addFilters(scope.createContext("/rpc", createAdminSecurityHandler( @@ -91,7 +83,9 @@ config.getFeedName().replace('_', '-')), config, sessionManager, secure))); addFilters(scope.createContext("/", - new RedirectHandler(contextPrefix + "/dashboard"))); + new RedirectHandler(dashboardContext.getPath()))); + + circularLogRpcMethod.start(); } private AdministratorSecurityHandler createAdminSecurityHandler( @@ -102,26 +96,7 @@ } public void stop() { - if (circularLogRpcMethod != null) { - circularLogRpcMethod.close(); - circularLogRpcMethod = null; - } - if (scope != null) { - scope.close(); - scope = null; - } - } - - public HttpServerScope getScope() { - return scope; - } - - public void addStatusSource(StatusSource source) { - monitor.addSource(source); - } - - public void clearStatusSources() { - monitor.clearSources(); + circularLogRpcMethod.close(); } private HttpContext addFilters(HttpContext context) { @@ -152,7 +127,7 @@ /** * Installs a log handler; to uninstall handler, call {@link #close}. */ - public CircularLogRpcMethod() { + public void start() { LogManager.getLogManager().getLogger("").addHandler(circularLog); } @@ -185,15 +160,25 @@ } static class StatusRpcMethod implements RpcHandler.RpcMethod { - private final StatusMonitor monitor; + private final List<StatusSource> sources; - public StatusRpcMethod(StatusMonitor monitor) { - this.monitor = monitor; + public StatusRpcMethod(List<StatusSource> sources) { + this.sources = Collections.unmodifiableList( + new ArrayList<StatusSource>(sources)); + } + + public Map<StatusSource, Status> retrieveStatuses() { + Map<StatusSource, Status> statuses + = new LinkedHashMap<StatusSource, Status>(sources.size() * 2); + for (StatusSource source : sources) { + statuses.put(source, source.retrieveStatus()); + } + return statuses; } @Override public Object run(List request) { - Map<StatusSource, Status> statuses = monitor.retrieveStatuses(); + Map<StatusSource, Status> statuses = retrieveStatuses(); List<Object> flatStatuses = new ArrayList<Object>(statuses.size()); // TODO(ejona): choose locale based on Accept-Languages. Locale locale = Locale.ENGLISH;
diff --git a/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java b/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java index 34b95db..fa66cbc 100644 --- a/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java +++ b/src/com/google/enterprise/adaptor/GsaCommunicationHandler.java
@@ -94,7 +94,10 @@ private ExecutorService backgroundExecutor; private DocIdCodec docIdCodec; private DocIdSender docIdSender; + private HttpServerScope dashboardScope; private Dashboard dashboard; + private final List<StatusSource> statusSources + = new CopyOnWriteArrayList<StatusSource>(); private SensitiveValueCodec secureValueCodec; private SamlIdentityProvider samlIdentityProvider; /** @@ -181,8 +184,13 @@ if (port != config.getServerPort()) { config.setValue("server.port", "" + port); } + int dashboardPort = dashboardServer.getAddress().getPort(); + if (dashboardPort != config.getServerDashboardPort()) { + config.setValue("server.dashboardPort", "" + dashboardPort); + } scope = new HttpServerScope(server, contextPrefix); + dashboardScope = new HttpServerScope(dashboardServer, contextPrefix); waiter = new ShutdownWaiter(); scheduleExecutor = Executors.newSingleThreadScheduledExecutor( @@ -221,9 +229,6 @@ docIdSender = new DocIdSender(fileMaker, fileSender, journal, config, adaptor); - dashboard = new Dashboard(config, this, journal, sessionManager, - secureValueCodec, adaptor); - // We are about to start the Adaptor, so anything available through // AdaptorContext or other means must be initialized at this point. Any // reference to 'adaptor' before this point must be done very carefully to @@ -349,7 +354,9 @@ TimeUnit.MILLISECONDS); } - dashboard.start(dashboardServer, contextPrefix); + dashboard = new Dashboard(config, this, journal, sessionManager, + secureValueCodec, adaptor, statusSources); + dashboard.start(dashboardScope); shuttingDownLatch = null; } @@ -610,10 +617,13 @@ private synchronized void realStop(long time, TimeUnit unit) { if (scope != null) { scope.close(); - } - if (dashboard != null) { - dashboard.clearStatusSources(); + scope = null; + + dashboardScope.close(); + dashboardScope = null; + dashboard.stop(); + dashboard = null; } if (scheduleExecutor != null) { scheduleExecutor.shutdownNow(); @@ -635,8 +645,6 @@ // AdaptorContext is usable until the very end. sendDocIdsFuture = null; scheduler = null; - scope = null; - dashboard = null; scheduleExecutor = null; backgroundExecutor = null; waiter = null; @@ -835,7 +843,7 @@ + "state, the adaptor is restarting."); HttpServer existingServer = scope.getHttpServer(); HttpServer existingDashboardServer - = dashboard.getScope().getHttpServer(); + = dashboardScope.getHttpServer(); stop(3, TimeUnit.SECONDS); try { start(existingServer, existingDashboardServer); @@ -871,7 +879,7 @@ if (afterInit) { throw new IllegalStateException("After init()"); } - dashboard.addStatusSource(source); + statusSources.add(source); } @Override
diff --git a/src/com/google/enterprise/adaptor/StatusMonitor.java b/src/com/google/enterprise/adaptor/StatusMonitor.java deleted file mode 100644 index 7d4454c..0000000 --- a/src/com/google/enterprise/adaptor/StatusMonitor.java +++ /dev/null
@@ -1,56 +0,0 @@ -// Copyright 2011 Google Inc. All Rights Reserved. -// -// 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.enterprise.adaptor; - -import java.util.*; -import java.util.concurrent.CopyOnWriteArrayList; - -/** - * Collection of various status LED sources. - * - * <p>This class is thread-safe. - */ -class StatusMonitor { - private List<StatusSource> sources = new CopyOnWriteArrayList<StatusSource>(); - - public Map<StatusSource, Status> retrieveStatuses() { - Map<StatusSource, Status> statuses - = new LinkedHashMap<StatusSource, Status>(sources.size() * 2); - for (StatusSource source : sources) { - statuses.put(source, source.retrieveStatus()); - } - return statuses; - } - - /** - * Add a {@code StatusSource}. - * - * @throws NullPointerException when {@code source} is {@code null} - */ - public void addSource(StatusSource source) { - if (source == null) { - throw new NullPointerException(); - } - sources.add(source); - } - - public void removeSource(StatusSource source) { - sources.remove(source); - } - - public void clearSources() { - sources.clear(); - } -}
diff --git a/test/com/google/enterprise/adaptor/DashboardTest.java b/test/com/google/enterprise/adaptor/DashboardTest.java index e0e0b7f..3ee23d9 100644 --- a/test/com/google/enterprise/adaptor/DashboardTest.java +++ b/test/com/google/enterprise/adaptor/DashboardTest.java
@@ -33,6 +33,7 @@ String golden = "Testing\n"; Dashboard.CircularLogRpcMethod method = new Dashboard.CircularLogRpcMethod(); + method.start(); try { Logger logger = Logger.getLogger(""); Level origLevel = logger.getLevel(); @@ -72,11 +73,11 @@ golden.add(goldenObj); } - StatusMonitor monitor = new StatusMonitor(); + List<StatusSource> sources = new ArrayList<StatusSource>(); Status status = new MockStatus(Status.Code.NORMAL, "fine"); MockStatusSource source = new MockStatusSource("mock", status); - monitor.addSource(source); - Dashboard.StatusRpcMethod method = new Dashboard.StatusRpcMethod(monitor); + sources.add(source); + Dashboard.StatusRpcMethod method = new Dashboard.StatusRpcMethod(sources); List list = (List) method.run(null); assertEquals(golden, list); }