refactor: use Apiman logger instead of System.err where possible
Closes
MODIFY
gateway/engine/core/src/main/java/io/apiman/gateway/engine/async/AsyncResultImpl.java
gateway/engine/core/src/main/java/io/apiman/gateway/engine/async/AsyncResultImpl.java
*/
package io.apiman.gateway.engine.async;
+import java.util.StringJoiner;
+
/**
* A simple implementation of the async result interface. Offers convenient
* creation of result instances.
@@ -112,4 +114,11 @@ public class AsyncResultImpl<T> implements IAsyncResult<T> {
return error;
}
+ @Override
+ public String toString() {
+ return new StringJoiner(", ", AsyncResultImpl.class.getSimpleName() + "[", "]")
+ .add("result=" + result)
+ .add("error=" + error)
+ .toString();
+ }
}

MODIFY
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/ConfigDrivenEngineFactory.java
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/ConfigDrivenEngineFactory.java
*/
package io.apiman.gateway.engine.impl;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.common.logging.IDelegateFactory;
import io.apiman.common.util.crypt.IDataEncrypter;
import io.apiman.gateway.engine.EngineConfigTuple;
@@ -44,10 +46,12 @@ import java.util.concurrent.atomic.AtomicInteger;
*/
public class ConfigDrivenEngineFactory extends AbstractEngineFactory {
- private IEngineConfig engineConfig;
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(ConfigDrivenEngineFactory.class);
+
+ private final IEngineConfig engineConfig;
private IAsyncResultHandler<Void> handler;
private boolean failed = false;
- private AtomicInteger asyncInitializeAwaiting = new AtomicInteger(0);
+ private final AtomicInteger asyncInitializeAwaiting = new AtomicInteger(0);
private boolean finishedLoading = false;
/**
@@ -81,7 +85,7 @@ public class ConfigDrivenEngineFactory extends AbstractEngineFactory {
return encrypter;
} catch (RuntimeException e) {
if ("No IDataEncrypter class configured.".equals(e.getMessage())) { //$NON-NLS-1$
- System.out.println("NOTE: No explicit Data Encrypter found. Falling back to the Default. [" + e.getMessage() + "]"); //$NON-NLS-1$ //$NON-NLS-2$
+ LOGGER.info("NOTE: No explicit Data Encrypter found. Falling back to the Default. [" + e.getMessage() + "]"); //$NON-NLS-1$ //$NON-NLS-2$
return new DefaultDataEncrypter();
} else {
throw e;
@@ -190,7 +194,7 @@ public class ConfigDrivenEngineFactory extends AbstractEngineFactory {
if (handler != null)
handler.handle(initResult);
} else {
- System.err.println("Failure occurred, but error handler was already invoked: " + initResult.getError().getCause()); //$NON-NLS-1$
+ LOGGER.error("Failure occurred, but error handler was already invoked", initResult.getError().getCause()); //$NON-NLS-1$
}
} else {
checkLoadingStatus();

MODIFY
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/DefaultJdbcComponent.java
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/DefaultJdbcComponent.java
package io.apiman.gateway.engine.impl;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.async.AsyncResultImpl;
import io.apiman.gateway.engine.async.IAsyncResultHandler;
import io.apiman.gateway.engine.components.IJdbcComponent;
@@ -26,7 +28,6 @@ import io.apiman.gateway.engine.components.jdbc.JdbcOptionsBean;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
-
import javax.sql.DataSource;
import com.zaxxer.hikari.HikariConfig;
@@ -39,7 +40,8 @@ import com.zaxxer.hikari.HikariDataSource;
*/
public class DefaultJdbcComponent implements IJdbcComponent {
- private Map<String, IJdbcClient> clients = new HashMap<>();
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(DefaultJdbcComponent.class);
+ private final Map<String, IJdbcClient> clients = new HashMap<>();
/**
* Constructor.
@@ -166,7 +168,7 @@ public class DefaultJdbcComponent implements IJdbcComponent {
// If not closed by now (since this is a synchronous implementation of the client
// interface) then the consumer messed up. We'll be nice and close it for them here.
if (jdbcConnection != null && !jdbcConnection.isClosed()) {
- System.err.print("NOTE: closing a JDBC connection that should have already been closed!"); //$NON-NLS-1$
+ LOGGER.warn("NOTE: closing a JDBC connection that should have already been closed! {0}", jdbcConnection); //$NON-NLS-1$
jdbcConnection.close();
}
} catch (Exception e) {

MODIFY
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/DefaultLdapSearchImpl.java
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/DefaultLdapSearchImpl.java
package io.apiman.gateway.engine.impl;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.async.AsyncResultImpl;
import io.apiman.gateway.engine.async.IAsyncHandler;
import io.apiman.gateway.engine.async.IAsyncResult;
@@ -38,11 +40,13 @@ import com.unboundid.ldap.sdk.SearchScope;
* @author Marc Savy {@literal <msavy@redhat.com>}
*/
public class DefaultLdapSearchImpl implements ILdapSearch {
- private LDAPConnection connection;
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(DefaultLdapSearchImpl.class);
+
+ private final LDAPConnection connection;
private IAsyncHandler<LdapException> ldapErrorHandler;
- private String searchDn;
- private String filter;
- private LdapSearchScope scope;
+ private final String searchDn;
+ private final String filter;
+ private final LdapSearchScope scope;
public DefaultLdapSearchImpl(String searchDn, String filter, LdapSearchScope scope, LDAPConnection connection) {
this.searchDn = searchDn;
@@ -59,8 +63,8 @@ public class DefaultLdapSearchImpl implements ILdapSearch {
result.handle(AsyncResultImpl.create(searchResults));
} catch (LDAPException e) {
if (ldapErrorHandler == null) {
- // TODO wire in logger
- System.err.println("LDAP Error Handler not set. Error may be swallowed; this is probably not what you intended.");
+ LOGGER.error("LDAP Error Handler not set. Error may be swallowed; "
+ + "this is probably not what you intended.", e);
}
ldapErrorHandler.handle(DefaultExceptionFactory.create(e));
} catch (Exception e) {

MODIFY
gateway/engine/filesystem/src/main/java/io/apiman/gateway/engine/filesystem/LocalFileRegistry.java
gateway/engine/filesystem/src/main/java/io/apiman/gateway/engine/filesystem/LocalFileRegistry.java
package io.apiman.gateway.engine.filesystem;
-import com.fasterxml.jackson.databind.ObjectMapper;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.async.IAsyncResultHandler;
import io.apiman.gateway.engine.beans.Api;
import io.apiman.gateway.engine.beans.Client;
import io.apiman.gateway.engine.beans.exceptions.RegistrationException;
import io.apiman.gateway.engine.filesystem.model.RegistryWrapper;
import io.apiman.gateway.engine.impl.InMemoryRegistry;
-import org.apache.commons.io.FileUtils;
-import org.apache.commons.lang3.StringUtils;
import java.io.File;
+import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.io.UncheckedIOException;
import java.util.Collection;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.StringUtils;
+
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;
@@ -28,7 +33,7 @@ import static java.util.stream.Collectors.toSet;
*/
public class LocalFileRegistry extends InMemoryRegistry {
static final String CONFIG_REGISTRY_PATH = "registry-path";
-
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(LocalFileRegistry.class);
private static final ObjectMapper mapper = new ObjectMapper();
private static final Object mutex = new Object();
private final File registryFile;
@@ -67,12 +72,13 @@ public class LocalFileRegistry extends InMemoryRegistry {
.collect(toMap(this::getApiIndex, identity())));
registryMap.putAll(wrapper.getClients().stream()
.collect(toMap(this::getClientIndex, identity())));
-
- } catch (Exception e) {
- throw new RuntimeException("Error reading registry from file: " + registryFile, e);
+ } catch (IOException ioe) {
+ throw new UncheckedIOException("Error reading registry from file: " + registryFile, ioe);
}
} else {
- System.err.println("Local registry file does not exists under path " + registryFile.getAbsolutePath());
+ LOGGER.warn("Local registry file does not exist under path {0}. "
+ + "It will be created when the first save operation occurs",
+ registryFile.getAbsolutePath());
}
map = registryMap;
}

MODIFY
gateway/engine/influxdb/src/main/java/io/apiman/gateway/engine/influxdb/InfluxDb09Metrics.java
gateway/engine/influxdb/src/main/java/io/apiman/gateway/engine/influxdb/InfluxDb09Metrics.java
*/
package io.apiman.gateway.engine.influxdb;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.IComponentRegistry;
import io.apiman.gateway.engine.IMetrics;
import io.apiman.gateway.engine.IRequiresInitialization;
@@ -40,6 +42,7 @@ import java.util.concurrent.CountDownLatch;
*/
@SuppressWarnings("nls")
public class InfluxDb09Metrics implements IMetrics, IRequiresInitialization {
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(InfluxDb09Metrics.class);
private static final String USER = "username";
private static final String PWORD = "password";
private static final String INFLUX_ENDPOINT = "endpoint";
@@ -54,15 +57,15 @@ public class InfluxDb09Metrics implements IMetrics, IRequiresInitialization {
DEFAULT_TAGS.put("generator", "apiman-gateway"); //$NON-NLS-2$
}
- private String dbName;
- private String retentionPolicy;
- private String seriesName;
- private String influxEndpoint;
+ private final String dbName;
+ private final String retentionPolicy;
+ private final String seriesName;
+ private final String influxEndpoint;
private IHttpClientComponent httpClient;
private InfluxDb09Driver driver;
- private String username;
- private String password;
+ private final String username;
+ private final String password;
/**
* Constructor.
@@ -109,11 +112,12 @@ public class InfluxDb09Metrics implements IMetrics, IRequiresInitialization {
(InfluxException result) -> {
if (result.isBadResponse()) {
IHttpClientResponse response = result.getResponse();
- System.err.println(String.format("Influx stats error. Code: %s with message: '%s'",
+ LOGGER.error(result,
+ "Influx stats error. Code: {0} with message: {1}",
response.getResponseCode(),
- response.getResponseMessage()));
+ response.getResponseMessage());
} else {
- System.err.println(result.getMessage());
+ LOGGER.error(result.getMessage(), result);
}
});
}
@@ -195,7 +199,7 @@ public class InfluxDb09Metrics implements IMetrics, IRequiresInitialization {
}
private String getOptionalString(Map<String, String> config, String key, String dValue) {
- return config.containsKey(key) ? config.get(key) : dValue;
+ return config.getOrDefault(key, dValue);
}
private List<String> listDatabases() {

MODIFY
gateway/engine/jdbc/src/main/java/io/apiman/gateway/engine/jdbc/JdbcMetrics.java
gateway/engine/jdbc/src/main/java/io/apiman/gateway/engine/jdbc/JdbcMetrics.java
package io.apiman.gateway.engine.jdbc;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.IComponentRegistry;
import io.apiman.gateway.engine.IMetrics;
import io.apiman.gateway.engine.metrics.RequestMetric;
@@ -43,6 +45,7 @@ import org.apache.commons.dbutils.QueryRunner;
*/
public class JdbcMetrics extends AbstractJdbcComponent implements IMetrics {
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(JdbcMetrics.class);
private static final int DEFAULT_QUEUE_SIZE = 10000;
protected IComponentRegistry componentRegistry;
@@ -143,11 +146,10 @@ public class JdbcMetrics extends AbstractJdbcComponent implements IMetrics {
);
} catch (InterruptedException ie) {
// This means that the thread was stopped.
+ LOGGER.trace("Metrics was stopped {0}", ie.getMessage());
} catch (Exception e) {
- // TODO better logging of this unlikely error
- System.err.println("Error adding metric to database:"); //$NON-NLS-1$
- e.printStackTrace();
- return;
+ LOGGER.error(e, "An unexpected error occurred when attempting to add a metric "
+ + "to the database. {0}", e.getMessage());
}
}
@@ -159,7 +161,8 @@ public class JdbcMetrics extends AbstractJdbcComponent implements IMetrics {
try {
queue.put(metric);
} catch (Exception e) {
- e.printStackTrace();
+ LOGGER.error(e, "An unexpected error occurred when attempting to enqueue a metric that "
+ + "was awaiting storage. The record may be lost {0}", e.getMessage());
}
}

MODIFY
gateway/engine/vertx-shareddata/src/main/java/io/apiman/gateway/engine/vertx/shareddata/SharedGlobalDataRegistry.java
gateway/engine/vertx-shareddata/src/main/java/io/apiman/gateway/engine/vertx/shareddata/SharedGlobalDataRegistry.java
@@ -29,33 +29,30 @@ import io.apiman.gateway.engine.beans.exceptions.ApiRetiredException;
import io.apiman.gateway.engine.beans.exceptions.ClientNotFoundException;
import io.apiman.gateway.engine.beans.exceptions.NoContractFoundException;
import io.apiman.gateway.engine.i18n.Messages;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
import io.vertx.core.AsyncResult;
import io.vertx.core.CompositeFuture;
import io.vertx.core.Future;
import io.vertx.core.Handler;
import io.vertx.core.Vertx;
+import io.vertx.core.logging.Logger;
+import io.vertx.core.logging.LoggerFactory;
import io.vertx.core.shareddata.AsyncMap;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-
/**
* @author Marc Savy {@literal <marc@rhymewithgravy.com>}
*/
@SuppressWarnings("nls")
public class SharedGlobalDataRegistry implements IRegistry {
- Vertx vertx;
- IEngineConfig vxConfig;
- Map<String, String> options;
- AsyncMap<String, Object> objectMap;
+ private static final Logger LOGGER = LoggerFactory.getLogger(SharedGlobalDataRegistry.class);
+ private AsyncMap<String, Object> objectMap;
public SharedGlobalDataRegistry(Vertx vertx, IEngineConfig vxConfig, Map<String, String> options) {
- this.vertx = vertx;
- this.vxConfig = vxConfig;
- this.options = options;
-
if (!vertx.isClustered()) {
throw new IllegalStateException(SharedGlobalDataRegistry.class.getCanonicalName() + " only works when operating in clustered mode!");
}
@@ -64,6 +61,9 @@ public class SharedGlobalDataRegistry implements IRegistry {
if (async.succeeded()) {
objectMap = async.result();
} else {
+ LOGGER.error("Problem getting cluster-wide Vert.x map: {}", async.cause(),
+ async.cause().getMessage());
+
throw new IllegalStateException(async.cause());
}
});

MODIFY
gateway/platforms/servlet/src/main/java/io/apiman/gateway/platforms/servlet/connectors/ssl/SSLSessionStrategyFactory.java
gateway/platforms/servlet/src/main/java/io/apiman/gateway/platforms/servlet/connectors/ssl/SSLSessionStrategyFactory.java
package io.apiman.gateway.platforms.servlet.connectors.ssl;
import io.apiman.common.config.options.TLSOptions;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import java.io.File;
import java.io.FileInputStream;
@@ -31,6 +33,7 @@ import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -57,10 +60,12 @@ import org.apache.http.util.Args;
* @author Marc Savy
*/
public class SSLSessionStrategyFactory {
- private SSLSessionStrategyFactory() {}
+ private static final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(SSLSessionStrategyFactory.class);
private static final HostnameVerifier ALLOW_ANY = new AllowAnyVerifier();
private static final TrustStrategy SELF_SIGNED = new TrustSelfSignedStrategy();
- private static String[] EMPTY = new String[]{};
+ private static final String[] EMPTY = new String[]{};
+
+ private SSLSessionStrategyFactory() {}
/**
* Convenience function parses map of options to generate {@link SSLSessionStrategy}.
@@ -208,7 +213,7 @@ public class SSLSessionStrategyFactory {
HostnameVerifier hostnameVerifier = allowAnyHostname ? ALLOW_ANY :
SSLConnectionSocketFactory.getDefaultHostnameVerifier();
PrivateKeyStrategy privateKeyStrategy = keyAliases == null ? null : new SelectByAlias(keyAliases);
- boolean clientAuth = keyStore == null ? false : true;
+ boolean clientAuth = keyStore != null;
SSLContextBuilder builder = SSLContexts.custom();
@@ -241,7 +246,7 @@ public class SSLSessionStrategyFactory {
* @return the ssl strategy
*/
public static SSLSessionStrategy buildUnsafe() {
- System.err.println("ATTENTION: SSLSessionStrategy will trust *any* certificate." //$NON-NLS-1$
+ LOGGER.warn("ATTENTION: SSLSessionStrategy will trust *any* certificate." //$NON-NLS-1$
+ " This is extremely unsafe for production. Caveat utilitor!"); //$NON-NLS-1$
try {
@@ -322,12 +327,10 @@ public class SSLSessionStrategyFactory {
}
private static final class SelectByAlias implements PrivateKeyStrategy {
- private Set<String> keyAliases = new HashSet<>();
+ private final Set<String> keyAliases = new HashSet<>();
public SelectByAlias(String[] keyAliases) {
- for (String k : keyAliases) {
- this.keyAliases.add(k);
- }
+ Collections.addAll(this.keyAliases, keyAliases);
}
@Override
@@ -349,11 +352,8 @@ public class SSLSessionStrategyFactory {
throws NoSuchAlgorithmException, KeyStoreException, CertificateException, IOException {
Args.notNull(file, "Truststore file"); //$NON-NLS-1$
final KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType());
- final FileInputStream instream = new FileInputStream(file);
- try {
+ try (FileInputStream instream = new FileInputStream(file)) {
trustStore.load(instream, tsp);
- } finally {
- instream.close();
}
return builder.loadTrustMaterial(trustStore, trustStrategy);
}
@@ -366,11 +366,8 @@ public class SSLSessionStrategyFactory {
KeyStoreException, UnrecoverableKeyException, CertificateException, IOException {
Args.notNull(file, "Keystore file"); //$NON-NLS-1$
final KeyStore identityStore = KeyStore.getInstance(KeyStore.getDefaultType());
- final FileInputStream instream = new FileInputStream(file);
- try {
+ try (FileInputStream instream = new FileInputStream(file)) {
identityStore.load(instream, ksp);
- } finally {
- instream.close();
}
return builder.loadKeyMaterial(identityStore, kp, privateKeyStrategy);
}

ADD
gateway/platforms/vertx3/vertx3/src/main/java/io/apiman/gateway/platforms/vertx3/helpers/AsyncHelper.java
gateway/platforms/vertx3/vertx3/src/main/java/io/apiman/gateway/platforms/vertx3/helpers/AsyncHelper.java
refactor: use Apiman logger instead of System.err where possible

Closes #1161