Fix review findings

This commit is contained in:
Sebastian Sdorra
2020-11-19 10:21:17 +01:00
parent b6c5a253cb
commit 982743e827
3 changed files with 63 additions and 23 deletions

View File

@@ -32,6 +32,7 @@ import org.apache.shiro.authc.AuthenticationException;
import org.apache.shiro.subject.Subject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.ExceptionWithContext;
import sonia.scm.NotFoundException;
import sonia.scm.repository.RepositoryHookType;
import sonia.scm.repository.api.HgHookMessage;
@@ -110,6 +111,9 @@ class DefaultHookHandler implements HookHandler {
} catch (NotFoundException ex) {
LOG.warn("could not find repository with id {}", request.getRepositoryId(), ex);
return error("repository not found");
} catch (ExceptionWithContext ex) {
LOG.debug("scm exception on hook occurred", ex);
return error(context, ex);
} catch (Exception ex) {
LOG.warn("unknown error on hook occurred", ex);
return error(context, ex);

View File

@@ -58,25 +58,6 @@ public class HookServer implements AutoCloseable {
this.handlerFactory = handlerFactory;
}
private ExecutorService createAcceptor() {
return Executors.newSingleThreadExecutor(
createThreadFactory("HgHookAcceptor")
);
}
private ExecutorService createWorkerPool() {
return Executors.newCachedThreadPool(
createThreadFactory("HgHookWorker-%d")
);
}
@Nonnull
private ThreadFactory createThreadFactory(String hgHookAcceptor) {
return new ThreadFactoryBuilder()
.setNameFormat(hgHookAcceptor)
.build();
}
public int start() throws IOException {
acceptor = createAcceptor();
workerPool = createWorkerPool();
@@ -126,15 +107,42 @@ public class HookServer implements AutoCloseable {
return new ServerSocket(0, 0, InetAddress.getLoopbackAddress());
}
private ExecutorService createAcceptor() {
return Executors.newSingleThreadExecutor(
createThreadFactory("HgHookAcceptor")
);
}
private ExecutorService createWorkerPool() {
return Executors.newCachedThreadPool(
createThreadFactory("HgHookWorker-%d")
);
}
@Nonnull
private ThreadFactory createThreadFactory(String hgHookAcceptor) {
return new ThreadFactoryBuilder()
.setNameFormat(hgHookAcceptor)
.build();
}
@Override
public void close() throws IOException {
if (serverSocket != null) {
serverSocket.close();
}
public void close() {
closeSocket();
shutdown(acceptor);
shutdown(workerPool);
}
private void closeSocket() {
if (serverSocket != null) {
try {
serverSocket.close();
} catch (IOException ex) {
LOG.warn("failed to close server socket", ex);
}
}
}
private void shutdown(ExecutorService acceptor) {
if (acceptor != null) {
acceptor.shutdown();

View File

@@ -34,6 +34,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import sonia.scm.ExceptionWithContext;
import sonia.scm.NotFoundException;
import sonia.scm.repository.RepositoryHookType;
import sonia.scm.repository.api.HgHookMessage;
@@ -47,6 +48,7 @@ import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.net.Socket;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
@@ -150,6 +152,20 @@ class DefaultHookHandlerTest {
assertError(response, "Something went wrong");
}
@Test
void shouldHandleExceptionWithContext() throws IOException {
mockMessageProvider();
doThrow(new TestingException("Exception with Context"))
.when(hookEventFacade)
.handle("42");
DefaultHookHandler.Request request = createRequest(RepositoryHookType.POST_RECEIVE);
DefaultHookHandler.Response response = send(request);
assertError(response, "Exception with Context");
}
@Test
void shouldSendMessagesOnException() throws IOException {
HgHookMessageProvider messageProvider = new HgHookMessageProvider();
@@ -245,4 +261,16 @@ class DefaultHookHandlerTest {
return Sockets.read(new ByteArrayInputStream(output.toByteArray()), DefaultHookHandler.Response.class);
}
private static class TestingException extends ExceptionWithContext {
private TestingException(String message) {
super(Collections.emptyList(), message);
}
@Override
public String getCode() {
return "42";
}
}
}