Add possibility to override global configuration per remote
Benefits:
* connection parameters can be fine tuned per-remote (there is no need
to specify lengthy timeouts for every endpoint when only one is
particularly slow) - the following parameters could be redefined:
connectionTimeout
socketTimeout
maxTries
retryInterval
* as a bonus RemoteConfig contains the following methods public:
getGlobal (to return global configuration)
getEffective (to return effective configuration per remote)
getName (to return remote name so that further parameters can be
reached)
in order to enable further configuration extension in EventProcessor
implementations.
Change-Id: I3ade3cd5c103d5810ecc8dd3bc0446e9ac81abbd
Originally-authored-by: Stephen Elsemore <selsemore@collab.net>
Signed-off-by: Jacek Centkowski <jcentkowski@collab.net>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/webhooks/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/webhooks/Configuration.java
index 416b6ee..eb1fb8c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/webhooks/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/webhooks/Configuration.java
@@ -40,10 +40,10 @@
@Inject
protected Configuration(PluginConfigFactory config, @PluginName String pluginName) {
PluginConfig cfg = config.getFromGerritConfig(pluginName, true);
- connectionTimeout = getInt(cfg, "connectionTimeout", DEFAULT_TIMEOUT_MS);
- socketTimeout = getInt(cfg, "socketTimeout", DEFAULT_TIMEOUT_MS);
- maxTries = getInt(cfg, "maxTries", DEFAULT_MAX_TRIES);
- retryInterval = getInt(cfg, "retryInterval", DEFAULT_RETRY_INTERVAL);
+ connectionTimeout = getInt(cfg, RemoteConfig.CONNECTION_TIMEOUT, DEFAULT_TIMEOUT_MS);
+ socketTimeout = getInt(cfg, RemoteConfig.SOCKET_TIMEOUT, DEFAULT_TIMEOUT_MS);
+ maxTries = getInt(cfg, RemoteConfig.MAX_TRIES, DEFAULT_MAX_TRIES);
+ retryInterval = getInt(cfg, RemoteConfig.RETRY_INTERVAL, DEFAULT_RETRY_INTERVAL);
threadPoolSize = getInt(cfg, "threadPoolSize", DEFAULT_THREAD_POOL_SIZE);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/webhooks/HttpSession.java b/src/main/java/com/googlesource/gerrit/plugins/webhooks/HttpSession.java
index f6b2b93..55696b6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/webhooks/HttpSession.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/webhooks/HttpSession.java
@@ -19,6 +19,7 @@
import com.googlesource.gerrit.plugins.webhooks.HttpResponseHandler.HttpResult;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
@@ -31,9 +32,10 @@
this.httpClient = httpClient;
}
- HttpResult post(String endpoint, EventProcessor.Request request) throws IOException {
- HttpPost post = new HttpPost(endpoint);
+ HttpResult post(RemoteConfig remote, EventProcessor.Request request) throws IOException {
+ HttpPost post = new HttpPost(remote.getUrl());
post.addHeader("Content-Type", MediaType.JSON_UTF_8.toString());
+ post.setConfig(getConfig(remote));
request
.headers
.entrySet()
@@ -45,4 +47,12 @@
post.setEntity(new StringEntity(request.body, StandardCharsets.UTF_8));
return httpClient.execute(post, new HttpResponseHandler());
}
+
+ private RequestConfig getConfig(RemoteConfig remote) {
+ return RequestConfig.custom()
+ .setConnectTimeout(remote.getConnectionTimeout())
+ .setConnectionRequestTimeout(remote.getConnectionTimeout())
+ .setSocketTimeout(remote.getSocketTimeout())
+ .build();
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/webhooks/PostTask.java b/src/main/java/com/googlesource/gerrit/plugins/webhooks/PostTask.java
index fbc386a..45c42a0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/webhooks/PostTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/webhooks/PostTask.java
@@ -37,8 +37,7 @@
private final ScheduledExecutorService executor;
private final HttpSession session;
- private final Configuration cfg;
- private final String url;
+ private final RemoteConfig remote;
private final Supplier<Optional<EventProcessor.Request>> processor;
private int execCnt;
@@ -46,14 +45,12 @@
public PostTask(
@WebHooksExecutor ScheduledExecutorService executor,
HttpSession session,
- Configuration cfg,
EventProcessor processor,
@Assisted ProjectEvent event,
@Assisted RemoteConfig remote) {
this.executor = executor;
this.session = session;
- this.cfg = cfg;
- this.url = remote.getUrl();
+ this.remote = remote;
this.processor = Suppliers.memoize(() -> processor.process(event, remote));
}
@@ -62,7 +59,7 @@
}
private void reschedule() {
- executor.schedule(this, cfg.getRetryInterval(), TimeUnit.MILLISECONDS);
+ executor.schedule(this, remote.getRetryInterval(), TimeUnit.MILLISECONDS);
}
@Override
@@ -70,18 +67,18 @@
try {
Optional<EventProcessor.Request> content = processor.get();
if (!content.isPresent()) {
- log.debug("No content. Webhook [{}] skipped.", url);
+ log.debug("No content. Webhook [{}] skipped.", remote.getUrl());
return;
}
execCnt++;
- HttpResult result = session.post(url, content.get());
- if (!result.successful && execCnt < cfg.getMaxTries()) {
+ HttpResult result = session.post(remote, content.get());
+ if (!result.successful && execCnt < remote.getMaxTries()) {
logRetry(result.message);
reschedule();
}
} catch (IOException e) {
- if (isRecoverable(e) && execCnt < cfg.getMaxTries()) {
+ if (isRecoverable(e) && execCnt < remote.getMaxTries()) {
logRetry(e);
reschedule();
} else {
@@ -96,13 +93,13 @@
private void logRetry(String reason) {
if (log.isDebugEnabled()) {
- log.debug("Retrying {} in {}ms. Reason: {}", toString(), cfg.getRetryInterval(), reason);
+ log.debug("Retrying {} in {}ms. Reason: {}", toString(), remote.getRetryInterval(), reason);
}
}
private void logRetry(Throwable cause) {
if (log.isDebugEnabled()) {
- log.debug("Retrying {} in {}ms. Cause: {}", toString(), cfg.getRetryInterval(), cause);
+ log.debug("Retrying {} in {}ms. Cause: {}", toString(), remote.getRetryInterval(), cause);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/webhooks/RemoteConfig.java b/src/main/java/com/googlesource/gerrit/plugins/webhooks/RemoteConfig.java
index 431d286..cd11638 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/webhooks/RemoteConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/webhooks/RemoteConfig.java
@@ -25,12 +25,20 @@
public static final String REMOTE = "remote";
+ static final String CONNECTION_TIMEOUT = "connectionTimeout";
+ static final String SOCKET_TIMEOUT = "socketTimeout";
+ static final String MAX_TRIES = "maxTries";
+ static final String RETRY_INTERVAL = "retryInterval";
+
+ private final Configuration global;
private final Config config;
private final String url;
private final String name;
@Inject
- RemoteConfig(@Assisted("config") Config config, @Assisted("name") String name) {
+ RemoteConfig(
+ Configuration global, @Assisted("config") Config config, @Assisted("name") String name) {
+ this.global = global;
this.config = config;
this.name = name;
this.url = config.getString(REMOTE, name, "url");
@@ -44,9 +52,29 @@
return config.getStringList(REMOTE, name, "event");
}
+ public int getConnectionTimeout() {
+ return config.getInt(REMOTE, name, CONNECTION_TIMEOUT, global.getConnectionTimeout());
+ }
+
+ public int getSocketTimeout() {
+ return config.getInt(REMOTE, name, SOCKET_TIMEOUT, global.getSocketTimeout());
+ }
+
+ public int getMaxTries() {
+ return config.getInt(REMOTE, name, MAX_TRIES, global.getMaxTries());
+ }
+
+ public int getRetryInterval() {
+ return config.getInt(REMOTE, name, RETRY_INTERVAL, global.getRetryInterval());
+ }
+
// methods were added in order to make configuration
// extensible in EvenptProcessor implementations
- public Config getConfig() {
+ public Configuration getGlobal() {
+ return global;
+ }
+
+ public Config getEffective() {
return config;
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 88ecdba..ec2ad91 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -1,24 +1,6 @@
@PLUGIN@ Configuration
=========================
-The @PLUGIN@ plugin's per project configuration is stored in the
-`webhooks.config` file in project's `refs/meta/config` branch.
-For example, to propagate all events to `https://foo.org/gerrit-events`
-and propagate only `patchset-created` and `ref-updated` events to
-`https://bar.org/`:
-
-```
-[remote "foo"]
- url = https://foo.org/gerrit-events
-
-[remote "bar"]
- url = https://bar.org/
- event = patchset-created
- event = ref-updated
-```
-
-The configuration is inheritable.
-
Global @PLUGIN@ plugin configuration is stored in the `gerrit.config` file.
An example global @PLUGIN@ configuration section:
@@ -31,16 +13,26 @@
threadPoolSize = 3
```
-File 'webhooks.config'
-----------------------
+The @PLUGIN@ plugin's per project configuration is stored in the
+`@PLUGIN@.config` file in project's `refs/meta/config` branch.
+For example, to propagate all events to `https://foo.org/gerrit-events`
+and propagate only `patchset-created` and `ref-updated` events to
+`https://bar.org/`:
-remote.NAME.url
-: Address of the remote server to post events to.
+```
+[remote "foo"]
+ url = https://foo.org/gerrit-events
+ maxTries = 3
-remote.NAME.event
-: Type of the event which will be posted to the remote url. Multiple event
- types can be specified, listing event types which should be posted.
- When no event type is configured, all events will be posted.
+[remote "bar"]
+ url = https://bar.org/
+ event = patchset-created
+ event = ref-updated
+```
+
+The configuration is inheritable. Connection parameters
+`connectionTimeout`, `socketTimeout`, `maxTries` and `retryInterval`
+can be fine-tuned at remote level.
File 'gerrit.config'
--------------------
@@ -66,4 +58,34 @@
@PLUGIN@.threadPoolSize
: Maximum number of threads used to send events to the target instance.
- Defaults to 2.
\ No newline at end of file
+ Defaults to 2.
+
+File '@PLUGIN@.config'
+----------------------
+
+remote.NAME.url
+: Address of the remote server to post events to.
+
+remote.NAME.event
+: Type of the event which will be posted to the remote url. Multiple event
+ types can be specified, listing event types which should be posted.
+ When no event type is configured, all events will be posted.
+
+remote.NAME.connectionTimeout
+: Maximum interval of time in milliseconds the plugin waits for a connection
+ to the target instance. When not specified, the default value is derrived
+ from global configuration.
+
+remote.NAME.socketTimeout
+: Maximum interval of time in milliseconds the plugin waits for a response from the
+ target instance once the connection has been established. When not specified,
+ the default value is derrived from global configuration.
+
+remote.NAME.maxTries
+: Maximum number of times the plugin should attempt when posting an event to
+ the target url. Setting this value to 0 will disable retries. When not
+ specified, the default value is derrived from global configuration.
+
+remote.NAME.retryInterval
+: The interval of time in milliseconds between the subsequent auto-retries.
+ When not specified, the default value is derrived from global configuration.
\ No newline at end of file
diff --git a/src/test/java/com/googlesource/gerrit/plugins/webhooks/PostTaskTest.java b/src/test/java/com/googlesource/gerrit/plugins/webhooks/PostTaskTest.java
index f52969a..e602177 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/webhooks/PostTaskTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/webhooks/PostTaskTest.java
@@ -62,11 +62,11 @@
@Before
public void setup() {
- when(cfg.getRetryInterval()).thenReturn(RETRY_INTERVAL);
- when(cfg.getMaxTries()).thenReturn(MAX_TRIES);
+ when(remote.getRetryInterval()).thenReturn(RETRY_INTERVAL);
+ when(remote.getMaxTries()).thenReturn(MAX_TRIES);
when(remote.getUrl()).thenReturn(WEBHOOK_URL);
when(processor.process(eq(projectCreated), eq(remote))).thenReturn(Optional.of(content));
- task = new PostTask(executor, session, cfg, processor, projectCreated, remote);
+ task = new PostTask(executor, session, processor, projectCreated, remote);
}
@Test
@@ -79,35 +79,35 @@
@Test
public void noRescheduleOnSuccess() throws IOException {
- when(session.post(eq(WEBHOOK_URL), eq(content))).thenReturn(OK_RESULT);
+ when(session.post(eq(remote), eq(content))).thenReturn(OK_RESULT);
task.run();
verifyZeroInteractions(executor);
}
@Test
public void noRescheduleOnNonRecoverableException() throws IOException {
- when(session.post(eq(WEBHOOK_URL), eq(content))).thenThrow(SSLException.class);
+ when(session.post(eq(remote), eq(content))).thenThrow(SSLException.class);
task.run();
verifyZeroInteractions(executor);
}
@Test
public void rescheduleOnError() throws IOException {
- when(session.post(eq(WEBHOOK_URL), eq(content))).thenReturn(ERR_RESULT);
+ when(session.post(eq(remote), eq(content))).thenReturn(ERR_RESULT);
task.run();
verify(executor, times(1)).schedule(task, RETRY_INTERVAL, TimeUnit.MILLISECONDS);
}
@Test
public void rescheduleOnRecoverableException() throws IOException {
- when(session.post(eq(WEBHOOK_URL), eq(content))).thenThrow(IOException.class);
+ when(session.post(eq(remote), eq(content))).thenThrow(IOException.class);
task.run();
verify(executor, times(1)).schedule(task, RETRY_INTERVAL, TimeUnit.MILLISECONDS);
}
@Test
public void keepReschedulingMaxTriesTimes() throws IOException {
- when(session.post(eq(WEBHOOK_URL), eq(content))).thenThrow(IOException.class);
+ when(session.post(eq(remote), eq(content))).thenThrow(IOException.class);
when(executor.schedule(task, RETRY_INTERVAL, TimeUnit.MILLISECONDS))
.then(
new Answer<Void>() {
