Fix javadoc issue and produce javadoc artifact during normal builds (#…

…594)

- fix javadoc issues (errors and warnings) to be compliant with JDK16
- produce javadoc artifact during normal build (instead of waiting for a release build)
- fail on error and warnings
- disable fail on warnings on JDK 15+ until code is fully documented (warnings are emitted when a public/protected method is not documented)

Addresses issue #588.

pom.xml

@@ -56,6 +56,10 @@
         <nexus-staging-maven-plugin.version>1.6.8</nexus-staging-maven-plugin.version>
 
         <checkstyle.version>8.45</checkstyle.version>
+
+        <!-- maven-javadoc-plugin configuration -->
+        <maven.javadoc.failOnError>true</maven.javadoc.failOnError>
+        <maven.javadoc.failOnWarnings>true</maven.javadoc.failOnWarnings>
     </properties>
 
     <licenses>
@@ -301,6 +305,18 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-javadoc-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>jar</goal>
+                        </goals>
+                        <phase>package</phase>
+                    </execution>
+                </executions>
+            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-release-plugin</artifactId>
@@ -449,12 +465,30 @@
                     <artifactId>maven-javadoc-plugin</artifactId>
                     <version>${maven-javadoc-plugin.version}</version>
                     <configuration>
-                        <doclint>none</doclint>
+                        <!-- Enable all validation checks except "accessibility"
+                         -->
+                        <doclint>all,-accessibility</doclint>
+
+                        <!-- Setup links to external api docs
+                         -->
                         <links>
                             <link>http://logback.qos.ch/apidocs</link>
                             <link>https://fasterxml.github.io/jackson-core/javadoc/2.10</link>
                             <link>https://fasterxml.github.io/jackson-databind/javadoc/2.10</link>
                         </links>
+
+                        <!-- Explicitly set version. This should help to get rid of the following
+                             warning when building the project with JDK 11+:
+                             
+                                The code being documented uses modules but the packages defined in
+                                https://docs.oracle.com/javase/8/docs/api/ are in the unnamed module.
+                         -->
+                        <source>${java.version}</source>
+
+                        <!-- Shuts off non-error and non-warning messages, leaving only the
+                             warnings and errors appear.
+                         -->
+                        <quiet>true</quiet>
                     </configuration>
                 </plugin>
 
@@ -533,6 +567,19 @@
                 </plugins>
             </build>
         </profile>
+
+        <profile>
+            <id>java15</id>
+            <activation>
+                <jdk>[1.15,)</jdk>
+            </activation>
+            <properties>
+                <!-- A warning is emitted when javadoc is missing on a public/protected method.
+                     Temporarily disable failure in this case until the code is fully documented.
+                -->
+                <maven.javadoc.failOnWarnings>false</maven.javadoc.failOnWarnings>
+            </properties>
+        </profile>
     </profiles>
 
     <distributionManagement>

src/main/java/net/logstash/logback/LogstashAccessFormatter.java

@@ -217,8 +217,9 @@ public boolean getLowerCaseHeaderNames() {
     }
 
     /**
-     * When true, names of headers will be written to JSON output in lowercase.
-     * @param lowerCaseHeaderNames When true, names of headers will be written to JSON output in lowercase.
+     * When true, names of headers will be written to JSON output in lower case.
+     * 
+     * @param lowerCaseHeaderNames When true, names of headers will be written to JSON output in lower case.
      */
     public void setLowerCaseHeaderNames(boolean lowerCaseHeaderNames) {
         this.requestHeadersProvider.setLowerCaseHeaderNames(lowerCaseHeaderNames);

src/main/java/net/logstash/logback/LogstashFormatter.java

@@ -349,17 +349,15 @@ public String getMessageSplitRegex() {
     /**
      * Write the message as a JSON array by splitting the message text using the specified regex.
      *
-     * <p>
-     * The allowed values are:
+     * <p>The allowed values are:
      * <ul>
      *     <li>Null/Empty : Disable message splitting. This is also the default behavior.</li>
      *     <li>Any valid regex : Use the specified regex.</li>
-     *     <li><tt>SYSTEM</tt> : Use the system-default line separator.</li>
-     *     <li><tt>UNIX</tt> : Use <tt>\n</tt>.</li>
-     *     <li><tt>WINDOWS</tt> : Use <tt>\r\n</tt>.</li>
+     *     <li>{@code SYSTEM} : Use the system-default line separator.</li>
+     *     <li>{@code UNIX} : Use {@code \n}.</li>
+     *     <li>{@code WINDOWS} : Use {@code \r\n}.</li>
      * </ul>
-     * </p>
-     * <p>
+     *
      * For example, if this parameter is set to the regex {@code #+}, then the logging statement:
      * <pre>
      * log.info("First line##Second line###Third line")
@@ -376,7 +374,6 @@ public String getMessageSplitRegex() {
      *     ...
      * }
      * </pre>
-     * </p>
      *
      * @param messageSplitRegex The regex used to split the message text
      */

src/main/java/net/logstash/logback/appender/AbstractLogstashTcpSocketAppender.java

@@ -1058,8 +1058,10 @@ public SocketFactory getSocketFactory() {
     }
 
     /**
-     * Used to create client {@link Socket}s to which to communicate.
-     * By default, it is the system default SocketFactory.
+     * Set the {@link SocketFactory} used to create client {@link Socket}s to which to communicate.
+     * Use {@code null} to use the system default SocketFactory.
+     * 
+     * @param socketFactory the socket factory to use to create connections with remote destinations.
      */
     public void setSocketFactory(SocketFactory socketFactory) {
         this.socketFactory = socketFactory;
@@ -1076,6 +1078,8 @@ public void setSocketFactory(SocketFactory socketFactory) {
      * <p>
      *
      * For example, "host1.domain.com,host2.domain.com:5560"
+     * 
+     * @param destination comma-separated list of destinations in the form of {@code hostName[:portNumber]}
      */
     public void addDestination(final String destination) throws IllegalArgumentException {
 
@@ -1086,6 +1090,8 @@ public void addDestination(final String destination) throws IllegalArgumentExcep
 
     /**
      * Adds the given destinations to the list of potential destinations.
+     * 
+     * @param destinations the {@link InetSocketAddress} to add to the list of valid destinations
      */
     public void addDestinations(InetSocketAddress... destinations) throws IllegalArgumentException  {
         if (destinations == null) {
@@ -1109,6 +1115,9 @@ public void addDestinations(InetSocketAddress... destinations) throws IllegalArg
     /**
      * Returns the host string from the given destination,
      * avoiding a DNS hit if possible.
+     * 
+     * @param destination the {@link InetSocketAddress} to get the host string from
+     * @return the host string of the given destination
      */
     protected String getHostString(InetSocketAddress destination) {
 
@@ -1136,6 +1145,8 @@ protected List<Object> getThreadNameFormatParams() {
 
     /**
      * Return the destinations in which to attempt to send logs.
+     * 
+     * @return an ordered list of {@link InetSocketAddress} representing the configured destinations
      */
     public List<InetSocketAddress> getDestinations() {
         return Collections.unmodifiableList(destinations);
@@ -1145,6 +1156,8 @@ public List<InetSocketAddress> getDestinations() {
      * Time period for which to wait after failing to connect to all servers,
      * before attempting to reconnect.
      * Default is {@value #DEFAULT_RECONNECTION_DELAY} milliseconds.
+     * 
+     * @param delay the reconnection delay
      */
     public void setReconnectionDelay(Duration delay) {
         if (delay == null || delay.getMilliseconds() <= 0) {
@@ -1165,6 +1178,7 @@ public Duration getReconnectionDelay() {
      * this will set its {@link PreferPrimaryDestinationConnectionStrategy#setSecondaryConnectionTTL(Duration)}.
      *
      * @see PreferPrimaryDestinationConnectionStrategy#setSecondaryConnectionTTL(Duration)
+     * @param secondaryConnectionTTL the TTL of a connection when connected to a secondary destination
      * @throws IllegalStateException if the {@link #connectionStrategy} is not a {@link PreferPrimaryDestinationConnectionStrategy}
      */
     public void setSecondaryConnectionTTL(Duration secondaryConnectionTTL) {
@@ -1184,6 +1198,8 @@ public Duration getSecondaryConnectionTTL() {
 
     /**
      * Socket connection timeout in milliseconds.
+     * 
+     * @param acceptConnectionTimeout connection timeout in milliseconds
      */
     void setAcceptConnectionTimeout(int acceptConnectionTimeout) {
         this.acceptConnectionTimeout = acceptConnectionTimeout;
@@ -1200,13 +1216,18 @@ public int getWriteBufferSize() {
      * If less than or equal to zero, buffering the output stream will be disabled.
      * If buffering is disabled, the writer thread can slow down, but
      * it will also can prevent dropping events in the buffer on flaky connections.
+     * 
+     * @param writeBufferSize the write buffer size in bytes
      */
    public void setWriteBufferSize(int writeBufferSize) {
         this.writeBufferSize = writeBufferSize;
     }
 
     /**
      * Returns the maximum number of events in the queue.
+     * Alias for {@link #getRingBufferSize()}.
+     * 
+     * @return the size of the ring buffer
      */
     public int getQueueSize() {
         return getRingBufferSize();
@@ -1232,6 +1253,8 @@ public SSLConfiguration getSsl() {
      * Set this to non-null to use SSL.
      * See <a href="http://logback.qos.ch/manual/usingSSL.html"> the logback manual</a>
      * for details on how to configure SSL for a client.
+     * 
+     * @param sslConfiguration the SSL configuration
      */
     public void setSsl(SSLConfiguration sslConfiguration) {
         this.sslConfiguration = sslConfiguration;
@@ -1246,6 +1269,8 @@ public Duration getKeepAliveDuration() {
      * order to keep the connection alive.
      *
      * When null, no keepAlive messages will be sent.
+     * 
+     * @param keepAliveDuration duration between consecutive keep alive messages
      */
     public void setKeepAliveDuration(Duration keepAliveDuration) {
         this.keepAliveDuration = keepAliveDuration;
@@ -1260,13 +1285,15 @@ public String getKeepAliveMessage() {
      *
      * The following values have special meaning:
      * <ul>
-     * <li><tt>null</tt> or empty string = no keep alive.</li>
-     * <li>"<tt>SYSTEM</tt>" = operating system new line (default).</li>
-     * <li>"<tt>UNIX</tt>" = unix line ending (\n).</li>
-     * <li>"<tt>WINDOWS</tt>" = windows line ending (\r\n).</li>
+     * <li>{@code null} or empty string = no keep alive.</li>
+     * <li>"{@code SYSTEM}" = operating system new line (default).</li>
+     * <li>"{@code UNIX}" = unix line ending (\n).</li>
+     * <li>"{@code WINDOWS}" = windows line ending (\r\n).</li>
      * </ul>
      * <p>
      * Any other value will be used as-is.
+     * 
+     * @param keepAliveMessage the keep alive message
      */
     public void setKeepAliveMessage(String keepAliveMessage) {
         this.keepAliveMessage = SeparatorParser.parseSeparator(keepAliveMessage);
@@ -1288,6 +1315,8 @@ public Charset getKeepAliveCharset() {
     /**
      * The charset to use when writing the {@link #keepAliveMessage}.
      * Defaults to UTF-8.
+     * 
+     * @param keepAliveCharset charset encoding for the keep alive message
      */
     public void setKeepAliveCharset(Charset keepAliveCharset) {
         this.keepAliveCharset = keepAliveCharset;
@@ -1310,6 +1339,8 @@ public void setKeepAliveCharset(Charset keepAliveCharset) {
      * The third argument (%3$s) is the string hostname of the currently connected destination.
      * The fourth argument (%4$d) is the numerical port of the currently connected destination.
      * Other arguments can be made available by subclasses.
+     * 
+     * @param threadNameFormat thread name format pattern
      */
     @Override
     public void setThreadNameFormat(String threadNameFormat) {
@@ -1346,15 +1377,15 @@ public Duration getWriteTimeout() {
      *
      * <p>Used to detect connections where the receiver stops reading.</p>
      *
-     * <p>Note that since a blocking java socket output stream
-     * does not have a concept of a write timeout,
-     * a task will be scheduled on the {@link #getExecutorService()}
-     * with the same frequency as the write timeout
-     * in order to detect stuck writes.
+     * <p>Note that since a blocking java socket output stream does not have a concept
+     * of a write timeout, a task will be scheduled with the same frequency as the write
+     * timeout in order to detect stuck writes.
      * It is recommended to use longer write timeouts (e.g. &gt; 30s, or minutes),
      * rather than short write timeouts, so that this task does not execute too frequently.
      * Also, this approach means that it could take up to two times the write timeout
      * before a write timeout is detected.</p>
+     * 
+     * @param writeTimeout the write timeout
      */
     public void setWriteTimeout(Duration writeTimeout) {
         this.writeTimeout = writeTimeout == null

src/main/java/net/logstash/logback/appender/AbstractLogstashUdpSocketAppender.java

@@ -140,7 +140,7 @@ public String getHost() {
     /**
      * Just an alias for syslogHost (since that name doesn't make much sense here)
      * 
-     * @param host
+     * @param host the name of the the host where log output should go
      */
     public void setHost(String host) {
         setSyslogHost(host);

src/main/java/net/logstash/logback/appender/AsyncDisruptorAppender.java

@@ -627,6 +627,8 @@ public String getThreadNameFormat() {
      * The first argument (%1$s) is the string appender name.
      * The second argument (%2$d) is the numerical thread index.
      * Other arguments can be made available by subclasses.
+     * 
+     * @param threadNameFormat the thread name format pattern
      */
     public void setThreadNameFormat(String threadNameFormat) {
         this.threadNameFormat = threadNameFormat;

src/main/java/net/logstash/logback/appender/WaitStrategyFactory.java

@@ -42,31 +42,32 @@ public class WaitStrategyFactory {
      * Creates a {@link WaitStrategy} from a string.
      * <p>
      * The following strategies are supported:
-     * <p>
      * <ul>
-     * <li><tt>blocking</tt> - {@link BlockingWaitStrategy}</li>
-     * <li><tt>busySpin</tt> - {@link BusySpinWaitStrategy}</li>
-     * <li><tt>liteBlocking</tt> - {@link LiteBlockingWaitStrategy}</li>
-     * <li><tt>sleeping{retries,sleepTimeNs}</tt> - {@link SleepingWaitStrategy}
-     *         - <tt>retries</tt> an integer number of times to spin before sleeping. (default = 200)
-     *           <tt>sleepTimeNs</tt> nanosecond time to sleep each iteration after spinning (default = 100)
+     * <li><code>blocking</code> - {@link BlockingWaitStrategy}</li>
+     * <li><code>busySpin</code> - {@link BusySpinWaitStrategy}</li>
+     * <li><code>liteBlocking</code> - {@link LiteBlockingWaitStrategy}</li>
+     * <li><code>sleeping{retries,sleepTimeNs}</code> - {@link SleepingWaitStrategy}
+     *         - <code>retries</code> an integer number of times to spin before sleeping. (default = 200)
+     *           <code>sleepTimeNs</code> nanosecond time to sleep each iteration after spinning (default = 100)
      * </li>
-     * <li><tt>yielding</tt> - {@link YieldingWaitStrategy}</li>
-     * <li><tt>phasedBackoff{spinTimeout,yieldTimeout,timeUnit,fallackStrategy}</tt> - {@link PhasedBackoffWaitStrategy}
-     *         - <tt>spinTimeout</tt> and <tt>yieldTimeout</tt> are long values.
-     *           <tt>timeUnit</tt> is a string name of one of the {@link TimeUnit} values.
-     *           <tt>fallbackStrategy</tt> is a wait strategy string (e.g. <tt>blocking</tt>).
+     * <li><code>yielding</code> - {@link YieldingWaitStrategy}</li>
+     * <li><code>phasedBackoff{spinTimeout,yieldTimeout,timeUnit,fallackStrategy}</code> - {@link PhasedBackoffWaitStrategy}
+     *         - <code>spinTimeout</code> and <code>yieldTimeout</code> are long values.
+     *           <code>timeUnit</code> is a string name of one of the {@link TimeUnit} values.
+     *           <code>fallbackStrategy</code> is a wait strategy string (e.g. <code>blocking</code>).
      * </li>
-     * <li><tt>timeoutBlocking{timeout,timeUnit}</tt> - {@link TimeoutBlockingWaitStrategy}
-     *         - <tt>timeout</tt> is a long value.
-     *           <tt>timeUnit</tt> is a string name of one of the {@link TimeUnit} values.
+     * <li><code>timeoutBlocking{timeout,timeUnit}</code> - {@link TimeoutBlockingWaitStrategy}
+     *         - <code>timeout</code> is a long value.
+     *           <code>timeUnit</code> is a string name of one of the {@link TimeUnit} values.
      * </li>
-     * <li><tt>liteTimeoutBlocking{timeout,timeUnit}</tt> - {@link LiteTimeoutBlockingWaitStrategy}
-     *         - <tt>timeout</tt> is a long value.
-     *           <tt>timeUnit</tt> is a string name of one of the {@link TimeUnit} values.
+     * <li><code>liteTimeoutBlocking{timeout,timeUnit}</code> - {@link LiteTimeoutBlockingWaitStrategy}
+     *         - <code>timeout</code> is a long value.
+     *           <code>timeUnit</code> is a string name of one of the {@link TimeUnit} values.
      * </li>
      * </ul>
      *
+     * @param waitStrategyType the name of the desired wait strategy
+     * @return a {@link WaitStrategy} instance or {@code null} if the supplied name is {@code null} or empty
      * @throws IllegalArgumentException if an unknown wait strategy type is given, or the parameters are unable to be parsed.
      */
     public static WaitStrategy createWaitStrategyFromString(String waitStrategyType) {

src/main/java/net/logstash/logback/appender/destination/DestinationParser.java

@@ -42,6 +42,10 @@ public class DestinationParser {
      * <p>
      *
      * If portNumber is not provided, then the given defaultPort will be used.
+     * 
+     * @param destinations comma-separated list of destinations in the form of {@code hostName[:portNumber]}
+     * @param defaultPort the port number to use when a destination does not specify one explicitly
+     * @return ordered list of {@link InetSocketAddress} instances
      */
     public static List<InetSocketAddress> parse(String destinations, int defaultPort) {
 

src/main/java/net/logstash/logback/appender/destination/PreferPrimaryDestinationConnectionStrategy.java

@@ -127,7 +127,8 @@ public Duration getSecondaryConnectionTTL() {
      * When the value is null (the default), the feature is disabled:
      * the appender will stay on the current destination until an error occurs.
      * 
-     * @param secondaryConnectionTTL
+     * @param secondaryConnectionTTL time to stay connected to a secondary connection
+     *                               before attempting to reconnect to the primary
      */
     public void setSecondaryConnectionTTL(Duration secondaryConnectionTTL) {
         if (secondaryConnectionTTL != null && secondaryConnectionTTL.getMilliseconds() <= 0) {