prepare 2.7.0 release (#73)

* add time threshold for backoff reset

* allow endpoint to be specified as either URI or HttpUrl

* add @SInCE

* add interface for customizing requests

* javadoc fixes

* add changelog for past releases

* remove JSR305

* replace SSL-specific config method with general-purpose HTTP config method

* make helper method static

* add end-to-end EventSource tests

* spacing

* omit default header value if there's a custom value

* avoid trailing period in logger name

* add 1.x branch

* update to OkHttp 4.x and Java 8

* javadoc fixes

* remove EventSource setters, + test improvements

* update Gradle release

* enable Github Pages

* skip tests in release

* add ability to force a stream restart; improve tests so we can test this

* revert whitespace change

* add 1.x branch

* bump OkHttp version to latest Java 7-compatible version

* prepare 1.10.2 release (#43)

* Releasing version 1.10.2

* Gradle update + build fixes

* add ability to force a stream restart; improve tests so we can test this (#29)

* prepare 1.11.0 release (#44)

* Releasing version 1.11.0

* Gradle release fixes

* update to okhttp 4.5.0

* longer timeout for cleaner shutdown of test servers

* fix Gradle scopes

* allow setting specific thread priority

* remove misleading logging & unnecessary backoff, improve tests (#34)

* known issue with onClose() - add comment, disable test assertions

* allow caller to specify a custom logger instead of SLF4J (#32)

* add method for changing base name of SLF4J logger

* enable coverage reports in CI, improve CI to test all supported Java versions

* rm inapplicable CI copy-paste

* another CI fix

* add checkstyle config

* fix jitter calculation when upper bound is a power of 2

* prepare 1.11.1 release (#49)

* Releasing version 1.11.1

* misc coverage + test improvements, add CI enforcement of coverage (#39)

* fix shutdown state logic, simplify code paths (#40)

* Fix Java 7 compatibility.

* add OpenJDK 7 build + fix test race condition + javadoc fix (#42)

* prepare 1.11.2 release (#56)

* Releasing version 1.11.2

* update Gradle to 6.8.3

* Kotlinize build script

* fix logic for shutting down after an unrecoverable error

* use newer HTTP test helpers

* use Releaser v2 config + newer CI images (#47)

* use new stream-reading implementation to support CR-only line endings

* make buffer size configurable

* rm usage that's not allowed in Java 8

* add Guava test dependency

* add code coverage ovverride

* implement contract tests (#48)

* use Gradle 7

* Bounded queues for the EventHandler thread (#58)

* Bounded queue for the EventHandler thread

The unbounded queue fronting the 'event' thread can cause trouble when the
EventHandler is unable to keep up with the workload. This can lead to heap
exhaustion, GC issues and failure modes that are generally considered "bad".

Band-aid over this with a semaphore to limit the number of tasks in the queue.
The semaphore is opt-in and disabled by default to avoid any nasty surprises
for folks upgrading.

Also add 'EventSource.awaitClosed()' to allow users to wait for underlying
thread pools to completely shut down. We can't know if it's safe to clean
up resources used by the EventHandler thread if we can't be certain that it
has completely terminated.

* Address checkstyle griping in StubServer

* Fix JavaDoc issue

* Tighten up exception handling

Co-authored-by: Eli Bishop <[email protected]>

* update @SInCE

* test Java 17 in CI (#51)

* improve tests for AsyncEventHandler and EventSource.awaitClosed (#52)

* add streaming data mode for very large events (#53)

* add option to ensure that expected fields are always read

* add Gradle option to suppress kotlin-stdlib in our pom

* update okhttp to 4.9.3

* use LaunchDarkly logging facade

* rm unused

* misc fixes

* improve javadoc links

* consistently use placeholders instead of concatenation in log output

Co-authored-by: Eli Bishop <[email protected]>
Co-authored-by: LaunchDarklyCI <[email protected]>
Co-authored-by: LaunchDarklyCI <[email protected]>
Co-authored-by: Gavin Whelan <[email protected]>
Co-authored-by: LaunchDarklyReleaseBot <[email protected]>
Co-authored-by: Tom Lee <[email protected]>

Author: 6 people

build.gradle.kts

@@ -1,6 +1,7 @@
 import java.time.Duration
 import org.gradle.api.tasks.testing.logging.TestExceptionFormat
 import org.gradle.external.javadoc.CoreJavadocOptions
+import org.gradle.external.javadoc.StandardJavadocDocletOptions
 
 // These values come from gradle.properties
 val ossrhUsername: String by project
@@ -60,14 +61,18 @@ java {
 }
 
 object Versions {
+    const val launchdarklyLogging = "1.1.0"
     const val okhttp = "4.9.3"
     const val slf4j = "1.7.22"
 }
 
 dependencies {
+    api("com.launchdarkly:launchdarkly-logging:${Versions.launchdarklyLogging}")
     api("com.squareup.okhttp3:okhttp:${Versions.okhttp}")
     api("org.slf4j:slf4j-api:${Versions.slf4j}")
-    testImplementation("ch.qos.logback:logback-classic:1.1.9")
+    // SLF4J is no longer referenced directly by okhttp-eventsource, but since the default behavior is
+    // to use the SLF4J adapter from com.launchdarkly.logging, we are still retaining the dependency
+    // here to make sure it is in the classpath.
     testImplementation("org.mockito:mockito-core:1.10.19")
     testImplementation("com.launchdarkly:test-helpers:1.0.0")
     testImplementation("com.google.guava:guava:30.1-jre")
@@ -90,6 +95,14 @@ tasks.javadoc.configure {
     // See JDK-8200363 (https://bugs.openjdk.java.net/browse/JDK-8200363)
     // for information about the -Xwerror option.
     (options as CoreJavadocOptions).addStringOption("Xwerror")
+
+    // The following should allow hyperlinks to com.launchdarkly.logging classes to go to
+    // the correct external URLs
+    if (options is StandardJavadocDocletOptions) {
+        (options as StandardJavadocDocletOptions).links(
+            "https://javadoc.io/doc/com.launchdarkly/launchdarkly-logging/${Versions.launchdarklyLogging}"
+        )
+    }
 }
 
 tasks.test.configure {
@@ -122,6 +135,11 @@ tasks.jacocoTestCoverageVerification.configure {
             "EventSource.run()" to 3,
             "EventSource.Builder.createInitialClientBuilder()" to 1,
             "EventSource.Builder.defaultTrustManager()" to 2,
+            "EventSource.Builder.loggerBaseName(java.lang.String)" to 2,
+            "LoggerBridge.ChannelImpl.log(com.launchdarkly.logging.LDLogLevel, java.lang.String, java.lang.Object[])" to 7,
+            "LoggerBridge.ChannelImpl.log(com.launchdarkly.logging.LDLogLevel, java.lang.String, java.lang.Object)" to 7,
+            "LoggerBridge.ChannelImpl.log(com.launchdarkly.logging.LDLogLevel, java.lang.Object)" to 7,
+            "LoggerBridge.ChannelImpl.isEnabled(com.launchdarkly.logging.LDLogLevel)" to 1,
             "MessageEvent.getData()" to 2,
             "SLF4JLogger.error(java.lang.String)" to 2,
             "ModernTLSSocketFactory.createSocket(java.lang.String, int)" to 1,

contract-tests/service/build.gradle

@@ -22,7 +22,6 @@ application {
 
 ext.versions = [
     "gson": "2.7",
-    "logback": "1.1.3",
     "testHelpers": "1.1.0"
 ]
 
@@ -32,7 +31,6 @@ configurations {
 
 dependencies {
     implementation project(":eventsource")
-    implementation "ch.qos.logback:logback-classic:${versions.logback}"
     implementation "com.google.code.gson:gson:${versions.gson}"
     implementation "com.launchdarkly:test-helpers:${versions.testHelpers}"
 }

contract-tests/service/src/main/java/ssetest/StreamEntity.java

@@ -1,8 +1,7 @@
 package ssetest;
 
 import com.launchdarkly.eventsource.*;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import com.launchdarkly.logging.*;
 import java.net.URI;
 import java.time.Duration;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -16,19 +15,19 @@ public class StreamEntity implements EventHandler {
   private final EventSource stream;
   private final StreamOptions options;
   private final AtomicInteger callbackMessageCounter = new AtomicInteger(0);
-  private final Logger logger;
+  private final LDLogger logger;
   private volatile boolean closed;
 
-  public StreamEntity(TestService owner, String id, StreamOptions options) {
+  public StreamEntity(TestService owner, String id, StreamOptions options, LDLogAdapter logAdapter) {
     this.owner = owner;
     this.id = id;
     this.options = options;
 
-    this.logger = LoggerFactory.getLogger(options.tag);
+    this.logger = LDLogger.withAdapter(logAdapter, options.tag);
     logger.info("Opening stream to {}", options.streamUrl);
 
     EventSource.Builder eb = new EventSource.Builder(this, URI.create(options.streamUrl))
-      .loggerBaseName(options.tag + ".stream");
+      .logger(logger.subLogger("stream"));
     if (options.headers != null) {
       Headers.Builder hb = new Headers.Builder();
       for (String name: options.headers.keySet()) {

contract-tests/service/src/main/java/ssetest/TestService.java

@@ -1,6 +1,7 @@
 package ssetest;
 
 import com.google.gson.Gson;
+import com.launchdarkly.logging.*;
 import com.launchdarkly.testhelpers.httptest.*;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
@@ -15,14 +16,12 @@ public class TestService {
 
   final Gson gson = new Gson();
   final OkHttpClient client = new OkHttpClient();
+  final LDLogAdapter logAdapter = Logs.toStream(System.out);
 
   private final Map<String, StreamEntity> streams = new ConcurrentHashMap<String, StreamEntity>();
   private final AtomicInteger streamCounter = new AtomicInteger(0);
 
   public static void main(String[] args) {
-    // ((ch.qos.logback.classic.Logger)LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME)).setLevel(
-    //     Level.valueOf(config.logLevel.toUpperCase()));
-
     TestService service = new TestService();
 
     SimpleRouter router = new SimpleRouter()
@@ -61,7 +60,7 @@ private void postCreateStream(RequestContext ctx) {
     StreamOptions opts = readJson(ctx, StreamOptions.class);
 
     String streamId = String.valueOf(streamCounter.incrementAndGet());
-    StreamEntity stream = new StreamEntity(this, streamId, opts);
+    StreamEntity stream = new StreamEntity(this, streamId, opts, logAdapter);
 
     streams.put(streamId, stream);
 

contract-tests/service/src/main/resources/logback.xml

@@ -1,6 +1,8 @@
 package com.launchdarkly.eventsource;
 
 
+import com.launchdarkly.logging.LDLogger;
+
 import java.util.concurrent.Executor;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.Semaphore;
@@ -16,10 +18,10 @@
 final class AsyncEventHandler implements EventHandler {
   private final Executor executor;
   private final EventHandler eventSourceHandler;
-  private final Logger logger;
+  private final LDLogger logger;
   final Semaphore semaphore; // visible for tests
 
-  AsyncEventHandler(Executor executor, EventHandler eventSourceHandler, Logger logger, Semaphore semaphore) {
+  AsyncEventHandler(Executor executor, EventHandler eventSourceHandler, LDLogger logger, Semaphore semaphore) {
     this.executor = executor;
     this.eventSourceHandler = eventSourceHandler;
     this.logger = logger;

src/main/java/com/launchdarkly/eventsource/AsyncEventHandler.java

@@ -1,5 +1,7 @@
 package com.launchdarkly.eventsource;
 
+import com.launchdarkly.logging.LDLogger;
+
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -34,7 +36,7 @@ final class EventParser {
   private final ConnectionHandler connectionHandler;
   private final boolean streamEventData;
   private Set<String> expectFields;
-  private final Logger logger;
+  private final LDLogger logger;
   private final URI origin;
 
   private BufferedLineParser lineParser;
@@ -59,7 +61,7 @@ final class EventParser {
       int readBufferSize,
       boolean streamEventData,
       Set<String> expectFields,
-      Logger logger
+      LDLogger logger
       ) {
     this.lineParser = new BufferedLineParser(inputStream,
         readBufferSize < MIN_READ_BUFFER_SIZE ? MIN_READ_BUFFER_SIZE : readBufferSize);

src/main/java/com/launchdarkly/eventsource/EventParser.java

@@ -1,5 +1,8 @@
 package com.launchdarkly.eventsource;
 
+import com.launchdarkly.logging.LDLogger;
+import com.launchdarkly.logging.LDSLF4J;
+
 import java.io.Closeable;
 import java.io.EOFException;
 import java.io.IOException;
@@ -63,7 +66,7 @@
  * which allows for greater efficiency in some use cases but has some behavioral constraints.
  */
 public class EventSource implements Closeable {
-  final Logger logger; // visible for tests
+  final LDLogger logger; // visible for tests
 
   /**
    * The default value for {@link Builder#reconnectTime(Duration)}: 1 second.
@@ -124,7 +127,7 @@ public class EventSource implements Closeable {
     if (builder.logger == null) {
       String loggerName = (builder.loggerBaseName == null ? EventSource.class.getCanonicalName() : builder.loggerBaseName) +
           (name.isEmpty() ? "" : ("." + name));
-      this.logger = new SLF4JLogger(loggerName);
+      this.logger = LDLogger.withAdapter(LDSLF4J.adapter(), loggerName);
     } else {
       this.logger = builder.logger;
     }
@@ -182,7 +185,7 @@ public void start() {
       return;
     }
     logger.debug("readyState change: {} -> {}", RAW, CONNECTING);
-    logger.info("Starting EventSource client using URI: " + url);
+    logger.info("Starting EventSource client using URI: {}", url);
     streamExecutor.execute(this::run);
   }
 
@@ -282,7 +285,7 @@ private void closeCurrentStream(ReadyState previousState) {
       // Otherwise, an IllegalArgumentException "Unbalanced enter/exit" error is thrown by okhttp.
       // https://github.com/google/ExoPlayer/issues/1348
       call.cancel();
-      logger.debug("call cancelled", null);
+      logger.debug("call cancelled");
     }
   }
 
@@ -337,7 +340,7 @@ private int maybeReconnectDelay(int reconnectAttempts, long connectedTime) {
 
     try {
       Duration sleepTime = backoffWithJitter(counter);
-      logger.info("Waiting " + sleepTime.toMillis() + " milliseconds before reconnecting...");
+      logger.info("Waiting {} milliseconds before reconnecting...", sleepTime.toMillis());
       Thread.sleep(sleepTime.toMillis());
     } catch (InterruptedException ignored) { // COVERAGE: no way to cause this in unit tests
     }
@@ -575,7 +578,7 @@ public static final class Builder {
     private RequestBody body = null;
     private OkHttpClient.Builder clientBuilder;
     private int readBufferSize = DEFAULT_READ_BUFFER_SIZE;
-    private Logger logger = null;
+    private LDLogger logger = null;
     private String loggerBaseName = null;
     private int maxEventTasksInFlight = 0;
     private boolean streamEventData;
@@ -667,11 +670,11 @@ public Builder requestTransformer(RequestTransformer requestTransformer) {
     }
 
     /**
-     * Set the name for this EventSource client to be used when naming the logger and threadpools. This is mainly useful when
-     * multiple EventSource clients exist within the same process.
+     * Set the name for this EventSource client to be used when naming thread pools (and, possibly, the logger).
+     * This is mainly useful when multiple EventSource clients exist within the same process.
      * <p>
      * The name only affects logging when using the default SLF4J integration; if you have specified a custom
-     * {@link #logger(Logger)}, the name will not be included in log messages unless your logger implementation adds it.
+     * {@link #logger(LDLogger)}, the logging facade has its own way to specify a logger name. 
      *
      * @param name the name (without any whitespaces)
      * @return the builder
@@ -925,13 +928,51 @@ public Builder readBufferSize(int readBufferSize) {
     /**
      * Specifies a custom logger to receive EventSource logging.
      * <p>
+     * This has been superseded by {@link #logger(LDLogger)}. The
+     * <a href="https://github.com/launchdarkly/java-logging">com.launchdarkly.logging</a>
+     * facade used by that method provides many options for customizing logging behavior.
+     * The {@link Logger} interface defined by {@code okhttp-eventsource} will be removed
+     * in a future major version release.
+     * <p>
      * If you do not provide a logger, the default is to send log output to SLF4J.
      * 
      * @param logger a {@link Logger} implementation, or null to use the default (SLF4J)
      * @return the builder
      * @since 2.3.0
+     * @deprecated use {@link #logger(LDLogger)}
      */
+    @Deprecated
     public Builder logger(Logger logger) {
+      this.logger = logger == null ? null : LoggerBridge.wrapLogger(logger);
+      return this;
+    }
+
+    /**
+     * Specifies a custom logger to receive EventSource logging.
+     * <p>
+     * This method uses the {@link LDLogger} type from
+     * <a href="https://github.com/launchdarkly/java-logging">com.launchdarkly.logging</a>, a
+     * facade that provides several logging implementations as well as the option to forward
+     * log output to SLF4J or another framework. Here is an example of configuring it to use
+     * the basic console logging implementation, and to tag the output with the name "logname":
+     * <pre><code>
+     *   // import com.launchdarkly.logging.*;
+     *   
+     *   builder.logger(
+     *      LDLogger.withAdapter(Logs.basic(), "logname") 
+     *   );
+     * </code></pre>
+     * <p>
+     * If you do not provide a logger, the default is to send log output to SLF4J, and to use
+     * a logger name based on the {@link #loggerBaseName(String)} and {@link #name(String)}
+     * settings. In a future major version, the default behavior may be changed so that this
+     * library no longer has a mandatory dependency on SLF4J.
+     * 
+     * @param logger an {@link LDLogger} implementation, or null to use the default (SLF4J)
+     * @return the builder
+     * @since 2.7.0
+     */
+    public Builder logger(LDLogger logger) {
       this.logger = logger;
       return this;
     }
@@ -942,11 +983,23 @@ public Builder logger(Logger logger) {
      * The default is {@code com.launchdarkly.eventsource.EventSource}, plus any name suffix specified
      * by {@link #name(String)}. If you instead use {@link #logger(Logger)} to specify some other log
      * destination rather than SLF4J, this name is unused.
+     * <p>
+     * This method is now deprecated, because the logging facade used by {@link #logger(LDLogger)}
+     * makes it easy to set a logger name for SLF4J, as in this example:
+     * <pre><code>
+     *   // import com.launchdarkly.logging.*;
+     *   
+     *   builder.logger(
+     *      LDLogger.withAdapter(LDSLF4J.adapter(), "my.preferred.log.name") 
+     *   );
+     * </code></pre>
      * 
      * @param loggerBaseName the SLF4J logger name, or null to use the default
      * @return the builder
      * @since 2.3.0
+     * @deprecated use {@link #logger(LDLogger)}
      */
+    @Deprecated
     public Builder loggerBaseName(String loggerBaseName) {
       this.loggerBaseName = loggerBaseName;
       return this;

src/main/java/com/launchdarkly/eventsource/EventSource.java

@@ -1,12 +1,19 @@
 package com.launchdarkly.eventsource;
 
 /**
- * Interface for a custom logger that an application can provide to receive EventSource logging.
+ * Deprecated interface for a custom logger that an application can provide to receive EventSource logging.
+ * <p>
+ * This has been superseded by {@link EventSource.Builder#logger(com.launchdarkly.logging.LDLogger)},
+ * which uses the <a href="https://github.com/launchdarkly/java-logging">com.launchdarkly.logging</a>
+ * facade, providing many options for customizing logging behavior. The {@link Logger} interface
+ * defined by {@code okhttp-eventsource} will be removed in a future major version release
  * <p>
  * If you do not provide a logger, the default is to send log output to SLF4J.
  *
  * @since 2.3.0
+ * @deprecated use {@link EventSource.Builder#logger(com.launchdarkly.logging.LDLogger)}
  */
+@Deprecated
 public interface Logger {
   /**
    * Logs a debug message. Debug output includes verbose details that applications will normally