Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Consul HTTP Client with Interface Clients #840

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<spring-cloud-deployer.version>2.8.3</spring-cloud-deployer.version>
<spring-cloud-openfeign.version>4.2.0-SNAPSHOT</spring-cloud-openfeign.version>
<spring-cloud-stream.version>4.2.0-SNAPSHOT</spring-cloud-stream.version>
<testcontainers.version>1.17.6</testcontainers.version>
<testcontainers.version>1.20.1</testcontainers.version>
<mockserverclient.version>5.15.0</mockserverclient.version>
</properties>

Expand Down
5 changes: 0 additions & 5 deletions spring-cloud-consul-binder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-stream</artifactId>
</dependency>
<dependency>
<groupId>com.ecwid.consul</groupId>
<artifactId>consul-api</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

import com.ecwid.consul.v1.OperationException;
import com.ecwid.consul.v1.event.model.Event;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.cloud.consul.model.http.event.Event;
import org.springframework.integration.endpoint.MessageProducerSupport;

/**
Expand Down Expand Up @@ -103,11 +102,6 @@ public void getEvents() {
.build());
}
}
catch (OperationException e) {
if (logger.isErrorEnabled()) {
logger.error("Error getting consul events: " + e);
}
}
catch (Exception e) {
if (logger.isErrorEnabled()) {
logger.error("Error getting consul events: " + e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,9 @@

import java.util.Arrays;

import com.ecwid.consul.v1.ConsulClient;
import com.ecwid.consul.v1.QueryParams;
import com.ecwid.consul.v1.Response;
import com.ecwid.consul.v1.event.model.Event;
import com.ecwid.consul.v1.event.model.EventParams;

import org.springframework.cloud.consul.IConsulClient;
import org.springframework.cloud.consul.model.http.event.Event;
import org.springframework.http.ResponseEntity;
import org.springframework.integration.handler.AbstractMessageHandler;
import org.springframework.messaging.Message;

Expand All @@ -34,11 +31,11 @@
*/
public class ConsulSendingHandler extends AbstractMessageHandler {

private final ConsulClient consul;
private final IConsulClient consul;

private final String eventName;

public ConsulSendingHandler(ConsulClient consul, String eventName) {
public ConsulSendingHandler(IConsulClient consul, String eventName) {
this.consul = consul;
this.eventName = eventName;
}
Expand All @@ -56,8 +53,7 @@ protected void handleMessageInternal(Message<?> message) {

// TODO: support headers
// TODO: support consul event filters: NodeFilter, ServiceFilter, TagFilter
Response<Event> event = this.consul.eventFire(this.eventName, (String) payload, new EventParams(),
QueryParams.DEFAULT);
ResponseEntity<Event> event = this.consul.eventFire(this.eventName, (String) payload);
// TODO: return event?
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;

import com.ecwid.consul.v1.ConsulClient;
import com.ecwid.consul.v1.QueryParams;
import com.ecwid.consul.v1.Response;
import com.ecwid.consul.v1.event.EventListRequest;
import com.ecwid.consul.v1.event.model.Event;
import com.ecwid.consul.v1.event.model.EventParams;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.annotation.PostConstruct;

import org.springframework.cloud.consul.IConsulClient;
import org.springframework.cloud.consul.binder.config.ConsulBinderProperties;
import org.springframework.cloud.consul.model.http.ConsulHeaders;
import org.springframework.cloud.consul.model.http.event.Event;
import org.springframework.http.ResponseEntity;

/**
* @author Spencer Gibb
Expand All @@ -37,19 +35,19 @@ public class EventService {

protected ConsulBinderProperties properties;

protected ConsulClient consul;
protected IConsulClient consul;

protected ObjectMapper objectMapper = new ObjectMapper();

private AtomicReference<Long> lastIndex = new AtomicReference<>();

public EventService(ConsulBinderProperties properties, ConsulClient consul, ObjectMapper objectMapper) {
public EventService(ConsulBinderProperties properties, IConsulClient consul, ObjectMapper objectMapper) {
this.properties = properties;
this.consul = consul;
this.objectMapper = objectMapper;
}

public ConsulClient getConsulClient() {
public IConsulClient getConsulClient() {
return this.consul;
}

Expand All @@ -62,24 +60,25 @@ public Long getLastIndex() {
return this.lastIndex.get();
}

private void setLastIndex(Response<?> response) {
Long consulIndex = response.getConsulIndex();
private void setLastIndex(ResponseEntity<?> response) {
String indexHeader = response.getHeaders().getFirst(ConsulHeaders.ConsulIndex.getHeaderName());
Long consulIndex = indexHeader == null ? null : Long.parseLong(indexHeader);
if (consulIndex != null) {
this.lastIndex.set(response.getConsulIndex());
this.lastIndex.set(consulIndex);
}
}

public Event fire(String name, String payload) {
Response<Event> response = this.consul.eventFire(name, payload, new EventParams(), QueryParams.DEFAULT);
return response.getValue();
ResponseEntity<Event> response = this.consul.eventFire(name, payload);
return response.getBody();
}

public Response<List<Event>> getEventsResponse() {
return this.consul.eventList(EventListRequest.newBuilder().setQueryParams(QueryParams.DEFAULT).build());
public ResponseEntity<List<Event>> getEventsResponse() {
return this.consul.eventList();
}

public List<Event> getEvents() {
return getEventsResponse().getValue();
return getEventsResponse().getBody();
}

public List<Event> getEvents(Long lastIndex) {
Expand All @@ -100,14 +99,13 @@ public List<Event> watch(Long lastIndex) {
if (this.properties != null) {
eventTimeout = this.properties.getEventTimeout();
}
Response<List<Event>> watch = this.consul
.eventList(EventListRequest.newBuilder().setQueryParams(new QueryParams(eventTimeout, index)).build());
ResponseEntity<List<Event>> watch = this.consul.eventList(eventTimeout, index);
return filterEvents(readEvents(watch), lastIndex);
}

protected List<Event> readEvents(Response<List<Event>> response) {
protected List<Event> readEvents(ResponseEntity<List<Event>> response) {
setLastIndex(response);
return response.getValue();
return response.getBody();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

package org.springframework.cloud.consul.binder.config;

import com.ecwid.consul.v1.ConsulClient;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
import org.springframework.cloud.consul.ConditionalOnConsulEnabled;
import org.springframework.cloud.consul.IConsulClient;
import org.springframework.cloud.consul.binder.ConsulBinder;
import org.springframework.cloud.consul.binder.EventService;
import org.springframework.cloud.stream.binder.Binder;
Expand Down Expand Up @@ -52,7 +52,7 @@ public class ConsulBinderConfiguration {

@Bean
@ConditionalOnMissingBean
public EventService eventService(ConsulClient consulClient) {
public EventService eventService(IConsulClient consulClient) {
return new EventService(null/* consulBinderProperties */, consulClient, this.objectMapper);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@

package org.springframework.cloud.consul.binder;

import java.io.IOException;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;
import java.util.concurrent.TimeUnit;

import com.ecwid.consul.v1.ConsulClient;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import org.junit.Before;
import org.junit.Ignore;
Expand All @@ -29,6 +32,9 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.consul.ConsulAutoConfiguration;
import org.springframework.cloud.consul.ConsulProperties;
import org.springframework.cloud.consul.IConsulClient;
import org.springframework.cloud.consul.test.ConsulTestcontainers;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand Down Expand Up @@ -112,12 +118,16 @@ interface Events {
public static class Application {

@Bean
public ConsulClient consulClient() {
return new ConsulClient("localhost", 18500);
public IConsulClient consulClient()
throws CertificateException, KeyStoreException, IOException, NoSuchAlgorithmException {
ConsulProperties consulProperties = new ConsulProperties();
consulProperties.setHost("localhost");
consulProperties.setPort(18500);
return ConsulAutoConfiguration.createNewConsulClient(consulProperties);
}

@Bean
public EventService eventService(ConsulClient consulClient) {
public EventService eventService(IConsulClient consulClient) {
EventService eventService = mock(EventService.class);
when(eventService.getConsulClient()).thenReturn(consulClient);
return eventService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.springframework.cloud.consul.binder;

import com.ecwid.consul.v1.OperationException;
import org.junit.Test;

import static org.assertj.core.api.Assertions.fail;
Expand All @@ -31,7 +30,7 @@ public class ConsulInboundMessageProducerTests {
@Test
public void getEventsShouldNotThrowException() {
EventService eventService = mock(EventService.class);
when(eventService.watch()).thenThrow(new OperationException(500, "error", ""));
when(eventService.watch()).thenThrow(new RuntimeException("error"));

ConsulInboundMessageProducer producer = new ConsulInboundMessageProducer(eventService);

Expand Down
10 changes: 5 additions & 5 deletions spring-cloud-consul-config/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@
<artifactId>spring-boot-starter-actuator</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>com.ecwid.consul</groupId>
<artifactId>consul-api</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-consul-core</artifactId>
Expand Down Expand Up @@ -82,6 +77,11 @@
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.ecwid.consul</groupId>
<artifactId>consul-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>consul</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.atomic.AtomicBoolean;

import com.ecwid.consul.v1.ConsulClient;
import com.ecwid.consul.v1.QueryParams;
import com.ecwid.consul.v1.Response;
import com.ecwid.consul.v1.kv.model.GetValue;
import io.micrometer.core.annotation.Timed;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.cloud.consul.IConsulClient;
import org.springframework.cloud.consul.model.http.ConsulHeaders;
import org.springframework.cloud.consul.model.http.kv.GetValue;
import org.springframework.cloud.endpoint.event.RefreshEvent;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.ApplicationEventPublisherAware;
import org.springframework.context.SmartLifecycle;
import org.springframework.core.style.ToStringCreator;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.scheduling.TaskScheduler;
import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
import org.springframework.util.ObjectUtils;
Expand All @@ -51,7 +52,7 @@ public class ConfigWatch implements ApplicationEventPublisherAware, SmartLifecyc

private final ConsulConfigProperties properties;

private final ConsulClient consul;
private final IConsulClient consul;

private final TaskScheduler taskScheduler;

Expand All @@ -65,12 +66,12 @@ public class ConfigWatch implements ApplicationEventPublisherAware, SmartLifecyc

private ScheduledFuture<?> watchFuture;

public ConfigWatch(ConsulConfigProperties properties, ConsulClient consul,
public ConfigWatch(ConsulConfigProperties properties, IConsulClient consul,
LinkedHashMap<String, Long> initialIndexes) {
this(properties, consul, initialIndexes, getTaskScheduler());
}

public ConfigWatch(ConsulConfigProperties properties, ConsulClient consul,
public ConfigWatch(ConsulConfigProperties properties, IConsulClient consul,
LinkedHashMap<String, Long> initialIndexes, TaskScheduler taskScheduler) {
this.properties = properties;
this.consul = consul;
Expand Down Expand Up @@ -154,13 +155,15 @@ public void watchConfigKeyValues() {
aclToken = null;
}

Response<List<GetValue>> response = this.consul.getKVValues(context, aclToken,
new QueryParams(this.properties.getWatch().getWaitTime(), currentIndex));
ResponseEntity<List<GetValue>> response = this.consul.getKVValues(context, aclToken,
(long) properties.getWatch().getWaitTime(), currentIndex);

// if response.value == null, response was a 404, otherwise it was a
// 200, reducing churn if there wasn't anything
if (response.getValue() != null && !response.getValue().isEmpty()) {
Long newIndex = response.getConsulIndex();
if (HttpStatus.OK.isSameCodeAs(response.getStatusCode()) && response.hasBody()
&& !response.getBody().isEmpty()) {
String indexHeader = response.getHeaders().getFirst(ConsulHeaders.ConsulIndex.getHeaderName());
Long newIndex = indexHeader == null ? null : Long.parseLong(indexHeader);

if (newIndex != null && !newIndex.equals(currentIndex)) {
// don't publish the same index again, don't publish the first
Expand Down
Loading