Rettelser etter QA

Erlend Viddal · Erlend Viddal · commit 8fd917efe2ea · 2017-08-04T19:24:44.000+02:00
diff --git a/servicebuilder-core/src/main/java/no/obos/util/servicebuilder/ServiceConfig.java b/servicebuilder-core/src/main/java/no/obos/util/servicebuilder/ServiceConfig.java
@@ -11,7 +11,6 @@
 import no.obos.util.servicebuilder.model.PropertyProvider;
 import no.obos.util.servicebuilder.model.ServiceDefinition;
 import no.obos.util.servicebuilder.util.GuavaHelper;
-import org.glassfish.hk2.api.TypeLiteral;
 import org.glassfish.hk2.utilities.binding.AbstractBinder;
 import org.glassfish.jersey.server.ResourceConfig;
 
@@ -50,10 +49,6 @@ public <T> ServiceConfig bind(Class<T> toBind) {
         return bind(binder -> binder.bindAsContract(toBind));
     }
 
-    public <T> ServiceConfig bindParameterized(T toBind) {
-        return bind(binder -> binder.bind(toBind).to(new TypeLiteral<T>() {}));
-    }
-
     public ServiceConfig bind(Binder binder) {
         return withBinders(GuavaHelper.plus(binders, binder));
     }
diff --git a/servicebuilder-interfaces/src/main/java/no/obos/util/servicebuilder/annotations/Mq.java b/servicebuilder-interfaces/src/main/java/no/obos/util/servicebuilder/annotations/Mq.java
diff --git a/servicebuilder-interfaces/src/main/java/no/obos/util/servicebuilder/model/MessageHandler.java b/servicebuilder-interfaces/src/main/java/no/obos/util/servicebuilder/model/MessageHandler.java
@@ -4,5 +4,5 @@
  * Interface for handling messages to be implemented by application
  */
 public interface MessageHandler<T> {
-    void handle(T message, MessageMeta meta);
+    void handle(T message, MessageMetadata meta);
 }
diff --git a/servicebuilder-interfaces/src/main/java/no/obos/util/servicebuilder/model/MessageMetadata.java b/servicebuilder-interfaces/src/main/java/no/obos/util/servicebuilder/model/MessageMetadata.java
@@ -8,7 +8,7 @@
  */
 @Builder
 @ToString
-public class MessageMeta {
+public class MessageMetadata {
     public final String requestId;
     public final String sourceApp;
 }
diff --git a/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/addon/ActiveMqAddon.java b/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/addon/ActiveMqAddon.java
@@ -68,11 +68,7 @@ public ActiveMqAddon initialize(ServiceConfig serviceConfig) {
                 .password(password)
                 .build();
 
-        ActiveMqListener listener = ActiveMqListener.builder()
-                .mqHandlerForwarder(mqAddon.mqHandlerForwarder)
-                .activeMqConnectionProvider(connectionProvider)
-                .handlerDescriptions(mqAddon.handlers)
-                .build();
+        ActiveMqListener listener = new ActiveMqListener(connectionProvider, mqAddon.mqHandlerForwarder);
 
         return this
                 .withMqAddon(mqAddon)
@@ -123,8 +119,8 @@ public void addToJettyServer(JettyServer jettyServer) {
         mqAddon.handlers.forEach(handlerDescription -> {
             String queueInput = handlerDescription.messageDescription.getQueueName();
             String queueError = handlerDescription.messageDescription.getErrorQueueName();
-            ObosHealthCheckRegistry.registerActiveMqCheck("Input queue: " + queueInput + " on " + url, url, queueInput, handlerDescription.healthCheckEntriesMax, handlerDescription.healthCheckEntriesMax, user, password);
-            ObosHealthCheckRegistry.registerActiveMqCheck("Error queue: " + queueError + " on " + url, url, queueError, 1, 60, user, password);
+            ObosHealthCheckRegistry.registerActiveMqCheck("Input queue: " + queueInput + " on " + url, url, queueInput, handlerDescription.healthCheckEntriesMax, handlerDescription.healthCheckEntriesGrace, user, password);
+            ObosHealthCheckRegistry.registerActiveMqCheck("Error queue: " + queueError + " on " + url, url, queueError, 1, 1, user, password);
             ObosHealthCheckRegistry.registerCustomCheck("ActiveMqListener active", () ->
                     listener.isListenerActive()
                             ? ObosHealthCheckResult.ok()
diff --git a/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqConnectionProvider.java b/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqConnectionProvider.java
@@ -14,6 +14,7 @@
 import java.util.function.Function;
 
 import static no.obos.util.servicebuilder.mq.activemq.ActiveMqUtils.closeConnection;
+import static no.obos.util.servicebuilder.mq.activemq.ActiveMqUtils.closeConnectionNoException;
 
 /**
  * Handles connections to activeMQ so other classes wont have to deal with connection-specific information.
@@ -25,12 +26,12 @@ public class ActiveMqConnectionProvider {
     private final String user;
     private final String password;
 
-    public void startListenerSession(BiConsumer<Connection, Session> fun, ErrorCallback onError) {
+    public void startListenerSession(BiConsumer<Connection, Session> listener, ErrorCallback onError) {
         ActiveMQConnection connection = ActiveMqUtils.openConnection(user, password, url);
 
         ExceptionListener onErrorWithClose = (JMSException ex) -> {
             log.error("Problem with ActiveMQ session, closing connection and forwarding", ex);
-            closeConnection(connection);
+            closeConnectionNoException(connection);
             onError.onError();
         };
         try {
@@ -40,7 +41,7 @@ public void startListenerSession(BiConsumer<Connection, Session> fun, ErrorCallb
             throw new MessageQueueException(e);
         }
         Session session = ActiveMqUtils.startSession(connection);
-        fun.accept(connection, session);
+        listener.accept(connection, session);
     }
 
     public <T> T inSessionWithReturn(Function<Session, T> fun) {
diff --git a/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqListener.java b/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqListener.java
@@ -1,10 +1,9 @@
 package no.obos.util.servicebuilder.mq.activemq;
 
 import com.google.common.collect.ImmutableSet;
-import lombok.Builder;
 import lombok.Getter;
+import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import no.obos.util.servicebuilder.mq.HandlerDescription;
 import no.obos.util.servicebuilder.mq.MqHandlerForwarder;
 import no.obos.util.servicebuilder.mq.MqHandlerImpl;
 import no.obos.util.servicebuilder.mq.MqListener;
@@ -17,33 +16,26 @@
  * Handles connecting several listeners to activemq in single session, reconnection and status for monitoring.
  */
 @Slf4j
-@Builder
+@RequiredArgsConstructor
 public class ActiveMqListener implements MqListener {
     private final ActiveMqConnectionProvider activeMqConnectionProvider;
 
-    private final ImmutableSet<HandlerDescription<?>> handlerDescriptions;
     private final MqHandlerForwarder mqHandlerForwarder;
 
     @Getter
     private boolean listenerActive = false;
 
-    private boolean abort = false;
+    private boolean threadIsInterrupted = false;
 
     private Connection connection = null;
     private Session session = null;
 
-    ImmutableSet<MqHandlerImpl<?>> handlers;
-
-    public void setHandlers(Iterable<MqHandlerImpl<?>> handlers) {
-        this.handlers = ImmutableSet.copyOf(handlers);
-    }
-
-    public void startListener() {
+    public void startListener(ImmutableSet<MqHandlerImpl<?>> handlers) {
         log.debug("Starting listener...");
         if (listenerActive) {
             throw new RuntimeException("Multiple active sessions in same listener. Check if starting connection threw exception and ActiveMQ ActiveMQConnection.setExceptionListener() failed at the same time.");
         }
-        abort = false;
+        threadIsInterrupted = false;
         try {
             activeMqConnectionProvider.startListenerSession((connection, session) -> {
                 this.connection = connection;
@@ -77,10 +69,10 @@ private void restartListener(ImmutableSet<MqHandlerImpl<?>> handlers) {
         } catch (InterruptedException e) {
             log.info("Interrupted.");
             Thread.currentThread().interrupt();
-            abort = true;
+            threadIsInterrupted = true;
         }
-        if (! abort) {
-            startListener();
+        if (! threadIsInterrupted) {
+            startListener(handlers);
         }
     }
 
diff --git a/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqQueueListener.java b/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqQueueListener.java
@@ -45,13 +45,14 @@ private void handleMessage(Message message, Session session) {
             return;
         }
         TextMessage textMessage = (TextMessage) message;
-        String text = null;
+        String text;
         try {
             text = textMessage.getText();
         } catch (JMSException e) {
             log.error("Could not read text from message", e);
             toErrorQueue(message);
             ActiveMqUtils.commitSession(session);
+            return;
         }
         try {
             mqHandlerForwarder.forward(handler, text);
@@ -73,6 +74,7 @@ private void toErrorQueue(String text) {
             errorProducer.close();
         } catch (JMSException jmse) {
             log.error("Failed to create error message", jmse);
+            throw new RuntimeException("Error queueing error message, aborting");
         }
     }
 
@@ -85,6 +87,7 @@ private void toErrorQueue(Message message) {
             errorProducer.close();
         } catch (JMSException jmse) {
             log.error("Failed to create error message", jmse);
+            throw new RuntimeException("Error queueing error message, aborting");
         }
     }
 
diff --git a/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqUtils.java b/servicebuilder-mq-activemq/src/main/java/no/obos/util/servicebuilder/mq/activemq/ActiveMqUtils.java
@@ -61,6 +61,14 @@ static void closeConnection(ActiveMQConnection connection) {
         }
     }
 
+    static void closeConnectionNoException(ActiveMQConnection connection) {
+        try {
+            connection.close();
+        } catch (JMSException ex) {
+            log.error("Could not close session connection, continuing", ex);
+        }
+    }
+
     static void commitSession(Session session) {
         try {
             session.commit();
diff --git a/servicebuilder-mq-activemq/src/test/java/no/obos/util/servicebuilder/mq/addon/ActiveMqAddonSendReceiveTest.java b/servicebuilder-mq-activemq/src/test/java/no/obos/util/servicebuilder/mq/addon/ActiveMqAddonSendReceiveTest.java
@@ -13,10 +13,10 @@
 import no.obos.util.servicebuilder.addon.ObosLogFilterAddon;
 import no.obos.util.servicebuilder.model.Constants;
 import no.obos.util.servicebuilder.model.MessageDescription;
-import no.obos.util.servicebuilder.model.ServiceDefinition;
 import no.obos.util.servicebuilder.model.MessageHandler;
-import no.obos.util.servicebuilder.model.MessageMeta;
+import no.obos.util.servicebuilder.model.MessageMetadata;
 import no.obos.util.servicebuilder.model.MessageSender;
+import no.obos.util.servicebuilder.model.ServiceDefinition;
 import org.apache.activemq.broker.BrokerService;
 import org.junit.Assert;
 import org.junit.Test;
@@ -59,13 +59,13 @@ public void sendAndReceiveMessage() {
 
             MyMessageV1 expected = new MyMessageV1(LocalDate.now(), "brillefin");
             ArgumentCaptor<MyMessageV1> myMessageV1ArgumentCator = ArgumentCaptor.forClass(MyMessageV1.class);
-            ArgumentCaptor<MessageMeta> messageMetaArgumentCaptor = ArgumentCaptor.forClass(MessageMeta.class);
+            ArgumentCaptor<MessageMetadata> messageMetaArgumentCaptor = ArgumentCaptor.forClass(MessageMetadata.class);
             TestServiceRunner.defaults(serviceConfig)
                     .oneShotVoid(MyResource.class, it -> it.addToQueue(expected));
             verify(messageHandler).handle(myMessageV1ArgumentCator.capture(), messageMetaArgumentCaptor.capture());
 
             MyMessageV1 actual = myMessageV1ArgumentCator.getValue();
-            MessageMeta meta = messageMetaArgumentCaptor.getValue();
+            MessageMetadata meta = messageMetaArgumentCaptor.getValue();
 
             assertThat(actual).isEqualTo(expected);
             assertThat(meta.sourceApp).isEqualTo(MyServiceDefinition.instance.getName());
diff --git a/servicebuilder-mq-mockmq/src/main/java/no/obos/util/servicebuilder/mq/mock/MqMock.java b/servicebuilder-mq-mockmq/src/main/java/no/obos/util/servicebuilder/mq/mock/MqMock.java
@@ -70,7 +70,7 @@ public MqMock(
 
 
     @Override
-    public void setHandlers(Iterable<MqHandlerImpl<?>> handlers) {
+    public void startListener(ImmutableSet<MqHandlerImpl<?>> handlers) {
         this.handlers = ImmutableMap.copyOf(
                 stream(handlers)
                         .collect(Collectors.toMap(
@@ -79,6 +79,7 @@ public void setHandlers(Iterable<MqHandlerImpl<?>> handlers) {
                                 )
                         )
         );
+        startListener();
     }
 
     @Override
@@ -93,8 +94,7 @@ public void queueMessage(String messageText, String queue) {
         }
     }
 
-    @Override
-    public void startListener() {
+    private void startListener() {
         if (! handlers.isEmpty()) {
             running = true;
             Runnable runnable = () -> {
diff --git a/servicebuilder-mq/pom.xml b/servicebuilder-mq/pom.xml
@@ -24,6 +24,12 @@
             <artifactId>lombok</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>com.fasterxml.jackson.module</groupId>
+            <artifactId>jackson-module-jsonSchema</artifactId>
+            <version>${jackson.version}</version>
+        </dependency>
+
 
     </dependencies>
 
diff --git a/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/addon/MqAddon.java b/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/addon/MqAddon.java
@@ -43,8 +43,12 @@ public class MqAddon implements Addon {
     public final ImmutableSet<HandlerDescription<?>> handlers;
     @Wither(AccessLevel.PRIVATE)
     public final ImmutableSet<MessageDescription<?>> senders;
+
+    /**
+     * Used internally to communicate with mq implementation.
+     */
     @Wither(AccessLevel.PRIVATE)
-    public final MqHandlerForwarder mqHandlerForwarder;
+    final MqHandlerForwarder mqHandlerForwarder;
     @Wither(AccessLevel.PRIVATE)
     public final boolean swagger;
 
@@ -61,7 +65,7 @@ public Addon initialize(ServiceConfig serviceConfig) {
     public void addToJerseyConfig(JerseyConfig serviceConfig) {
         // Feature is used to start the listeners immediately once dependencies are bound
         serviceConfig.addRegistations(registrator -> registrator
-                .register(StartListenersFeature.class)
+                .register(StartListenerFeature.class)
         );
         serviceConfig.addBinder((binder) -> {
             binder.bind(MqSenderResolver.class).to(JustInTimeInjectionResolver.class);
@@ -98,6 +102,14 @@ public MqAddon send(ServiceDefinition serviceDefinition) {
         return this.withSenders(GuavaHelper.plusAll(senders, serviceDefinition.getHandledMessages()));
     }
 
+    /**
+     * Magic solution to inject senders based on generic message type. Uses hk2 just in time injection.
+     * Basically, when hk2 does not find a candidate for injection among bound classes, it asks any just
+     * in time injection resolvers if they have an implementation of the class.
+     * <p>
+     * Senders injected via this resolver is provided in a sender map given by the mq implementation addon
+     * (e.g. ActiveMqAddon). Thus the addon must bind the sender map.
+     */
     static class MqSenderResolver implements JustInTimeInjectionResolver {
         @Inject
         ServiceLocator serviceLocator;
@@ -117,7 +129,10 @@ public boolean justInTimeResolution(Injectee failedInjectionPoint) {
     }
 
 
-    private static class StartListenersFeature implements Feature {
+    /**
+     * Starts a new thread for the listener using MqListener provided by mq implementation.
+     */
+    private static class StartListenerFeature implements Feature {
         @Inject
         private ServiceLocator serviceLocator;
 
@@ -129,8 +144,7 @@ public boolean configure(FeatureContext context) {
             ImmutableSet<MqHandlerImpl<?>> handlers = mqAddon.handlers.stream()
                     .map(this::getHandlerImpl)
                     .collect(GuavaHelper.setCollector());
-            listener.setHandlers(handlers);
-            listener.startListener();
+            listener.startListener(handlers);
             return true;
         }
 
diff --git a/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqHandlerForwarder.java b/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqHandlerForwarder.java
@@ -5,7 +5,7 @@
 import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import no.obos.util.servicebuilder.model.MessageHandler;
-import no.obos.util.servicebuilder.model.MessageMeta;
+import no.obos.util.servicebuilder.model.MessageMetadata;
 import org.slf4j.MDC;
 
 import java.io.IOException;
@@ -56,15 +56,16 @@ private <T> MqMessage<T> parseMessage(HandlerDescription<T> handlerDescription,
     }
 
     private <T> void forwardMessage(MqHandlerImpl<T> handler, MqMessage<T> message) {
-        MessageMeta messageMeta = MessageMeta.builder()
+        log.info("Received message: {}", message.toString());
+        MessageMetadata messageMetadata = MessageMetadata.builder()
                 .requestId(message.requestId)
                 .sourceApp(message.sourceApp)
                 .build();
 
         MessageHandler<T> messageHandler = handler.messageHandler;
 
         try {
-            messageHandler.handle(message.content, messageMeta);
+            messageHandler.handle(message.content, messageMetadata);
         } catch (RuntimeException ex) {
             log.error("Exception during processing message.", ex);
             throw ex;
diff --git a/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqListener.java b/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqListener.java
@@ -1,11 +1,7 @@
 package no.obos.util.servicebuilder.mq;
 
-public interface MqListener {
-    /**
-     * Only do this once, and before starting listener threads. If not the dark gods of multithreading will bring horrible
-     * agony on you and your children and your childrens children.
-     */
-    void setHandlers(Iterable<MqHandlerImpl<?>> handlers);
+import com.google.common.collect.ImmutableSet;
 
-    void startListener();
+public interface MqListener {
+    void startListener(ImmutableSet<MqHandlerImpl<?>> handlers);
 }
diff --git a/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqMessage.java b/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqMessage.java
diff --git a/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqSwaggerDecorator.java b/servicebuilder-mq/src/main/java/no/obos/util/servicebuilder/mq/MqSwaggerDecorator.java

Original file line number	Diff line number	Diff line change
`@@ -4,5 +4,5 @@`
`4`	`4`	`* Interface for handling messages to be implemented by application`
`5`	`5`	`*/`
`6`	`6`	`public interface MessageHandler<T> {`
`7`		`- void handle(T message, MessageMeta meta);`
	`7`	`+ void handle(T message, MessageMetadata meta);`
`8`	`8`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`*/`
`9`	`9`	`@Builder`
`10`	`10`	`@ToString`
`11`		`-public class MessageMeta {`
	`11`	`+public class MessageMetadata {`
`12`	`12`	`public final String requestId;`
`13`	`13`	`public final String sourceApp;`
`14`	`14`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,13 +45,14 @@ private void handleMessage(Message message, Session session) {`
`45`	`45`	`return;`
`46`	`46`	`}`
`47`	`47`	`TextMessage textMessage = (TextMessage) message;`
`48`		`- String text = null;`
	`48`	`+ String text;`
`49`	`49`	`try {`
`50`	`50`	`text = textMessage.getText();`
`51`	`51`	`} catch (JMSException e) {`
`52`	`52`	`log.error("Could not read text from message", e);`
`53`	`53`	`toErrorQueue(message);`
`54`	`54`	`ActiveMqUtils.commitSession(session);`
	`55`	`+ return;`
`55`	`56`	`}`
`56`	`57`	`try {`
`57`	`58`	`mqHandlerForwarder.forward(handler, text);`
`@@ -73,6 +74,7 @@ private void toErrorQueue(String text) {`
`73`	`74`	`errorProducer.close();`
`74`	`75`	`} catch (JMSException jmse) {`
`75`	`76`	`log.error("Failed to create error message", jmse);`
	`77`	`+ throw new RuntimeException("Error queueing error message, aborting");`
`76`	`78`	`}`
`77`	`79`	`}`
`78`	`80`
`@@ -85,6 +87,7 @@ private void toErrorQueue(Message message) {`
`85`	`87`	`errorProducer.close();`
`86`	`88`	`} catch (JMSException jmse) {`
`87`	`89`	`log.error("Failed to create error message", jmse);`
	`90`	`+ throw new RuntimeException("Error queueing error message, aborting");`
`88`	`91`	`}`
`89`	`92`	`}`
`90`	`93`
Original file line number	Diff line number	Diff line change
`@@ -61,6 +61,14 @@ static void closeConnection(ActiveMQConnection connection) {`
`61`	`61`	`}`
`62`	`62`	`}`
`63`	`63`
	`64`	`+ static void closeConnectionNoException(ActiveMQConnection connection) {`
	`65`	`+ try {`
	`66`	`+ connection.close();`
	`67`	`+ } catch (JMSException ex) {`
	`68`	`+ log.error("Could not close session connection, continuing", ex);`
	`69`	`+ }`
	`70`	`+ }`
	`71`	`+`
`64`	`72`	`static void commitSession(Session session) {`
`65`	`73`	`try {`
`66`	`74`	`session.commit();`