better implementation of shutdown on startup error, fix jooby-project#451

jknack · jknack · commit 1cc04febc863 · 2016-09-05T17:37:02.000-03:00
diff --git a/jooby-maven-plugin/src/main/java/org/jooby/JoobyRunner.java b/jooby-maven-plugin/src/main/java/org/jooby/JoobyRunner.java
@@ -60,7 +60,9 @@ public void run(final String mainClass, final Consumer<Jooby> callback)
       Thread.currentThread().setContextClassLoader(local);
       Jooby app = (Jooby) local.loadClass(mainClass).newInstance();
       callback.accept(app);
-      app.start(routes);
+      if (routes != null) {
+        routes.accept(Jooby.exportRoutes(app));
+      }
     } finally {
       Thread.currentThread().setContextClassLoader(global);
     }
diff --git a/jooby-spec/src/main/java/org/jooby/spec/RouteProcessor.java b/jooby-spec/src/main/java/org/jooby/spec/RouteProcessor.java
@@ -77,13 +77,6 @@
  */
 public class RouteProcessor {
 
-  /**
-   * Force cancellation of startup. Internal use.
-   */
-  @SuppressWarnings("serial")
-  private static class StopEx extends RuntimeException {
-  }
-
   /** The logging system. */
   private final Logger log = LoggerFactory.getLogger(getClass());
 
@@ -184,19 +177,7 @@ public List<RouteSpec> compile(final Class<? extends Jooby> appClass,
    * @return List of route specs.
    */
   private List<RouteSpec> processInternal(final Jooby app, final Path srcdir, final Path outdir) {
-    List<Route.Definition> routes = new ArrayList<>();
-    try {
-      app.start(r -> {
-        routes.addAll(r);
-        throw new StopEx();
-      });
-    } catch (StopEx cause) {
-      log.trace("routes were collected successfully");
-    } catch (Throwable cause) {
-      log.error("Unable to get routes from {}", app, cause);
-      return Collections.emptyList();
-    }
-
+    List<Route.Definition> routes = Jooby.exportRoutes(app);
     return processInternal(app.getClass(), routes, srcdir, outdir);
   }
 
diff --git a/jooby/src/main/java/org/jooby/Jooby.java b/jooby/src/main/java/org/jooby/Jooby.java
@@ -66,6 +66,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
@@ -131,7 +132,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
 import com.google.inject.Binder;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
@@ -3497,54 +3497,31 @@ public static void run(final Class<? extends Jooby> app, final String... args) {
   }
 
   /**
-   * <h1>Bootstrap</h1>
-   * <p>
-   * The bootstrap process is defined as follows:
-   * </p>
-   * <h2>1. Configuration files (first-listed are higher priority)</h2>
-   * <ol>
-   * <li>System properties</li>
-   * <li>Application properties: {@code application.conf} or custom, see {@link #use(Config)}</li>
-   * <li>{@link Jooby.Module Modules} properties</li>
-   * </ol>
+   * Export routes from an application. Useful for route analysis, testing, debugging, etc...
    *
-   * <h2>2. Dependency Injection and {@link Jooby.Module modules}</h2>
-   * <ol>
-   * <li>An {@link Injector Guice Injector} is created.</li>
-   * <li>It calls to {@link Jooby.Module#configure(Env, Config, Binder)} for each module.</li>
-   * <li>At this point Guice is ready and all the services has been binded.</li>
-   * <li>A web server is started</li>
-   * </ol>
+   * @param app Application to extract/collect routes.
+   * @return Application routes.
    */
-  public void start() {
-    start(new String[0]);
-  }
+  public static List<Definition> exportRoutes(final Jooby app) {
+    @SuppressWarnings("serial")
+    class Success extends RuntimeException {
+      List<Definition> routes;
 
-  /**
-   * <h1>Bootstrap</h1>
-   * <p>
-   * The bootstrap process is defined as follows:
-   * </p>
-   * <h2>1. Configuration files (first-listed are higher priority)</h2>
-   * <ol>
-   * <li>System properties</li>
-   * <li>Application properties: {@code application.conf} or custom, see {@link #use(Config)}</li>
-   * <li>{@link Jooby.Module Modules} properties</li>
-   * </ol>
-   *
-   * <h2>2. Dependency Injection and {@link Jooby.Module modules}</h2>
-   * <ol>
-   * <li>An {@link Injector Guice Injector} is created.</li>
-   * <li>It calls to {@link Jooby.Module#configure(Env, Config, Binder)} for each module.</li>
-   * <li>At this point Guice is ready and all the services has been binded.</li>
-   * <li>A web server is started</li>
-   * </ol>
-   *
-   * @param routes Routes callback. Invoked once all app routes has been collected.
-   * @throws Throwable If something fails to start.
-   */
-  public void start(final Consumer<List<Route.Definition>> routes) throws Throwable {
-    start(new String[0], routes);
+      Success(final List<Route.Definition> routes) {
+        this.routes = routes;
+      }
+    }
+    List<Definition> routes = Collections.emptyList();
+    try {
+      app.start(new String[0], r -> {
+        throw new Success(r);
+      });
+    } catch (Success success) {
+      routes = success.routes;
+    } catch (Throwable x) {
+      logger(app).error("Unable to get routes from {}", app, x);
+    }
+    return routes;
   }
 
   /**
@@ -3566,12 +3543,9 @@ public void start(final Consumer<List<Route.Definition>> routes) throws Throwabl
    * <li>At this point Guice is ready and all the services has been binded.</li>
    * <li>A web server is started</li>
    * </ol>
-   *
-   * @param args Application arguments. Using the <code>name=value</code> format, except for
-   *        application.env where can be just: <code>myenv</code>.
    */
-  public void start(final String[] args) {
-    start(args, null);
+  public void start() {
+    start(new String[0]);
   }
 
   /**
@@ -3596,55 +3570,59 @@ public void start(final String[] args) {
    *
    * @param args Application arguments. Using the <code>name=value</code> format, except for
    *        application.env where can be just: <code>myenv</code>.
-   * @param routes Routes callback. Invoked once all app routes has been collected.
    */
-  public void start(final String[] args, final Consumer<List<Route.Definition>> routes) {
+  public void start(final String... args) {
     try {
-      long start = System.currentTimeMillis();
+      start(args, null);
+    } catch (Throwable x) {
+      stop(Optional.of(x));
+    }
+  }
 
-      started.set(true);
+  private void start(final String[] args, final Consumer<List<Route.Definition>> routes)
+      throws Throwable {
+    long start = System.currentTimeMillis();
 
-      // shutdown hook
-      Runtime.getRuntime().addShutdownHook(new Thread(() -> stop()));
+    started.set(true);
 
-      this.injector = bootstrap(args(args), routes);
+    this.injector = bootstrap(args(args), routes);
 
-      Config conf = injector.getInstance(Config.class);
+    // shutdown hook
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> stop()));
 
-      Logger log = LoggerFactory.getLogger(getClass());
-      log.debug("config tree:\n{}", configTree(conf.origin().description()));
+    Config conf = injector.getInstance(Config.class);
 
-      // start services
-      for (CheckedConsumer<Registry> onStart : this.onStart) {
-        onStart.accept(this);
-      }
+    Logger log = LoggerFactory.getLogger(getClass());
+    log.debug("config tree:\n{}", configTree(conf.origin().description()));
 
-      // route mapper
-      Set<Route.Definition> routeDefs = injector.getInstance(Route.KEY);
-      Set<WebSocket.Definition> sockets = injector.getInstance(WebSocket.KEY);
-      if (mapper != null) {
-        routeDefs.forEach(it -> it.map(mapper));
-      }
+    // start services
+    for (CheckedConsumer<Registry> onStart : this.onStart) {
+      onStart.accept(this);
+    }
 
-      // Start server
-      Server server = injector.getInstance(Server.class);
-      String serverName = server.getClass().getSimpleName().replace("Server", "").toLowerCase();
+    // route mapper
+    Set<Route.Definition> routeDefs = injector.getInstance(Route.KEY);
+    Set<WebSocket.Definition> sockets = injector.getInstance(WebSocket.KEY);
+    if (mapper != null) {
+      routeDefs.forEach(it -> it.map(mapper));
+    }
 
-      server.start();
-      long end = System.currentTimeMillis();
+    // Start server
+    Server server = injector.getInstance(Server.class);
+    String serverName = server.getClass().getSimpleName().replace("Server", "").toLowerCase();
 
-      log.info("[{}@{}]: Server started in {}ms\n\n{}\n",
-          conf.getString("application.env"),
-          serverName,
-          end - start,
-          new AppPrinter(routeDefs, sockets, conf));
+    server.start();
+    long end = System.currentTimeMillis();
 
-      boolean join = conf.hasPath("server.join") ? conf.getBoolean("server.join") : true;
-      if (join) {
-        server.join();
-      }
-    } catch (Throwable x) {
-      stop(Optional.of(x));
+    log.info("[{}@{}]: Server started in {}ms\n\n{}\n",
+        conf.getString("application.env"),
+        serverName,
+        end - start,
+        new AppPrinter(routeDefs, sockets, conf));
+
+    boolean join = conf.hasPath("server.join") ? conf.getBoolean("server.join") : true;
+    if (join) {
+      server.join();
     }
   }
 
@@ -4058,11 +4036,11 @@ private Injector bootstrap(final Config args,
     List<Object> bag = normalize(realbag, env, rm, prefix);
 
     // collect routes and fire route callback
-    List<Route.Definition> routes = bag.stream()
-        .filter(it -> it instanceof Route.Definition)
-        .map(it -> (Route.Definition) it)
-        .collect(Collectors.<Route.Definition> toList());
     if (rcallback != null) {
+      List<Route.Definition> routes = bag.stream()
+          .filter(it -> it instanceof Route.Definition)
+          .map(it -> (Route.Definition) it)
+          .collect(Collectors.<Route.Definition> toList());
       rcallback.accept(routes);
     }
 
@@ -4354,22 +4332,20 @@ public void stop() {
 
   private void stop(final Optional<Throwable> x) {
     if (started.compareAndSet(true, false)) {
-      Logger log = LoggerFactory.getLogger(getClass());
-      fireStop(injector, this, log, onStop);
+      Logger log = logger(this);
 
-      List<Throwable> cause = Lists.newArrayList();
-      x.ifPresent(cause::add);
-      try {
-        injector.getInstance(Server.class).stop();
-      } catch (Throwable ex) {
-        cause.add(ex);
-      }
-      if (cause.size() > 0) {
-        log.error("Shutdown after error", cause.get(0));
-      } else {
-        log.info("Shutdown successfully");
+      x.ifPresent(c -> log.error("An error occurred while starting the application:", c));
+      fireStop(injector, this, log, onStop);
+      if (injector != null) {
+        try {
+          injector.getInstance(Server.class).stop();
+        } catch (Throwable ex) {
+          log.debug("server.stop() resulted in exception", ex);
+        }
       }
       injector = null;
+
+      log.info("Stopped");
     }
   }
 
@@ -4676,4 +4652,8 @@ static String logback(final Config conf) {
     return logback;
   }
 
+  private static Logger logger(final Jooby app) {
+    return LoggerFactory.getLogger(app.getClass());
+  }
+
 }
diff --git a/jooby/src/test/java/org/jooby/JoobyRunTest.java b/jooby/src/test/java/org/jooby/JoobyRunTest.java
@@ -1,6 +1,5 @@
 package org.jooby;
 
-import static org.easymock.EasyMock.aryEq;
 import static org.easymock.EasyMock.expect;
 
 import java.util.Arrays;
@@ -27,13 +26,13 @@ public ArgEx(final String[] args) {
 
   public static class NoopApp extends Jooby {
     @Override
-    public void start(final String[] args) {
+    public void start(final String... args) {
     }
   }
 
   public static class NoopAppEx extends Jooby {
     @Override
-    public void start(final String[] args) {
+    public void start(final String... args) {
       throw new ArgEx(args);
     }
   }
@@ -49,7 +48,7 @@ public void runSupplier() throws Exception {
         })
         .expect(unit -> {
           Jooby jooby = unit.get(Jooby.class);
-          jooby.start(aryEq(args));
+          jooby.start(args);
         })
         .run(unit -> {
           Jooby.run(unit.get(Supplier.class), args);
@@ -67,7 +66,7 @@ public void runSupplierArg() throws Exception {
         })
         .expect(unit -> {
           Jooby jooby = unit.get(Jooby.class);
-          jooby.start(aryEq(args));
+          jooby.start(args);
         })
         .run(unit -> {
           Jooby.run(unit.get(Supplier.class), args);
diff --git a/jooby/src/test/java/org/jooby/JoobyTest.java b/jooby/src/test/java/org/jooby/JoobyTest.java
diff --git a/jooby/src/test/java/org/jooby/JsBootTest.java b/jooby/src/test/java/org/jooby/JsBootTest.java
