Skip to content

Commit

Permalink
Refactored and added tests
Browse files Browse the repository at this point in the history
  • Loading branch information
santanusinha committed Aug 22, 2024
1 parent daca695 commit d995b55
Show file tree
Hide file tree
Showing 42 changed files with 1,172 additions and 367 deletions.
21 changes: 21 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
<mockito.version>4.2.0</mockito.version>

<dropwizard.version>2.1.10</dropwizard.version>
<logback.version>1.2.12</logback.version>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -188,6 +189,26 @@
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M5</version>
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.3.1</version>
<dependencies>
<dependency>
<groupId>me.fabriciorby</groupId>
<artifactId>maven-surefire-junit5-tree-reporter</artifactId>
<version>1.3.0</version>
</dependency>
</dependencies>
<configuration>
<reportFormat>plain</reportFormat>
<consoleOutputReporter>
<disable>true</disable>
</consoleOutputReporter>
<statelessTestsetInfoReporter
implementation="org.apache.maven.plugin.surefire.extensions.junit5.JUnit5StatelessTestsetInfoTreeReporter">
</statelessTestsetInfoReporter>
</configuration>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
@Slf4j
public class ServiceFinderHub<T, R extends ServiceRegistry<T>> {
@Getter
private final AtomicReference<Map<Service, ServiceFinder<T, R>>> finders = new AtomicReference<>(new ConcurrentHashMap<>());
private final AtomicReference<Map<Service, ServiceFinder<T, R>>> finders =
new AtomicReference<>(new ConcurrentHashMap<>());
private final Lock updateLock = new ReentrantLock();
private final Condition updateCond = updateLock.newCondition();
private boolean updateAvailable = false;
Expand Down Expand Up @@ -75,9 +76,9 @@ public class ServiceFinderHub<T, R extends ServiceRegistry<T>> {
public ServiceFinderHub(
ServiceDataSource serviceDataSource,
ServiceFinderFactory<T, R> finderFactory
) {
) {
this(serviceDataSource, finderFactory,
HubConstants.SERVICE_REFRESH_DURATION_MS, HubConstants.HUB_REFRESH_DURATION_MS);
HubConstants.SERVICE_REFRESH_DURATION_MS, HubConstants.HUB_REFRESH_DURATION_MS);
}

public ServiceFinderHub(
Expand All @@ -90,9 +91,9 @@ public ServiceFinderHub(
this.serviceRefreshDurationMs = serviceRefreshDurationMs;
this.hubRefreshDurationMs = hubRefreshDurationMs;
this.refreshSignals.add(new ScheduledSignal<>("service-hub-updater",
() -> null,
Collections.emptyList(),
10_000));
() -> null,
Collections.emptyList(),
10_000));
}

public Optional<ServiceFinder<T, R>> finder(final Service service) {
Expand All @@ -110,7 +111,8 @@ public CompletableFuture<ServiceFinder<T, R>> buildFinder(final Service service)
updateAvailable();
waitTillServiceIsReady(service);
return finders.get().get(service);
} catch(Exception e) {
}
catch (Exception e) {
log.warn("Exception whiling building finder", e);
throw e;
}
Expand All @@ -133,12 +135,14 @@ public void stop() {
if (null != monitorFuture) {
try {
monitorFuture.cancel(true);
} catch (Exception e) {
}
catch (Exception e) {
log.warn("Error stopping service finder hub monitor: {}", e.getMessage());
}
}
log.info("Service finder hub stopped");
}

public void registerUpdateSignal(final Signal<Void> refreshSignal) {
refreshSignals.add(refreshSignal);
}
Expand All @@ -148,7 +152,8 @@ public void updateAvailable() {
updateLock.lock();
updateAvailable = true;
updateCond.signalAll();
} finally {
}
finally {
updateLock.unlock();
}
}
Expand All @@ -161,11 +166,13 @@ private void monitor() {
updateCond.await();
}
updateRegistry();
} catch (InterruptedException e) {
}
catch (InterruptedException e) {
log.info("Updater thread interrupted");
Thread.currentThread().interrupt();
break;
} finally {
}
finally {
updateAvailable = false;
updateLock.unlock();
}
Expand All @@ -181,7 +188,7 @@ private void updateRegistry() {
final Map<Service, ServiceFinder<T, R>> updatedFinders = new HashMap<>();
try {
val services = serviceDataSource.services();
if(services.isEmpty()) {
if (services.isEmpty()) {
log.debug("No services found for the service data source. Skipping update on the registry");
return;
}
Expand All @@ -200,9 +207,11 @@ private void updateRegistry() {
updatedFinders.putAll(newFinders);
updatedFinders.putAll(matchingServices);
finders.set(updatedFinders);
} catch (Exception e) {
}
catch (Exception e) {
log.error("Error updating service list. Will maintain older list", e);
} finally {
}
finally {
alreadyUpdating.set(false);
}
}
Expand All @@ -215,14 +224,20 @@ private void waitTillHubIsReady() {
waitTillServiceIsReady(service);
return null;
})).toArray(CompletableFuture[]::new)
);
);
try {
hubRefresher.get(hubRefreshDurationMs, TimeUnit.MILLISECONDS);
} catch (InterruptedException ie) {
}
catch (InterruptedException ie) {
Thread.currentThread().interrupt();
Exceptions.illegalState("Refresh interrupted");
}
catch (TimeoutException e) {
Exceptions
.illegalState("Couldn't perform hub refresh at this time. Refresh exceeded the start up time specified");
} catch (Exception e) {
.illegalState("Couldn't perform service hub refresh at this time. " +
"Refresh exceeded the start up time specified");
}
catch (Exception e) {
Exceptions
.illegalState("Couldn't perform hub refresh at this time", e);
}
Expand All @@ -238,7 +253,8 @@ private void waitTillServiceIsReady(Service service) {
.map(ServiceFinder::getServiceRegistry)
.map(ServiceRegistry::isRefreshed)
.orElse(false));
} catch (Exception e) {
}
catch (Exception e) {
Exceptions
.illegalState("Could not perform initial state for service: " + service.getServiceName(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
"iterations" : 4,
"threads" : 1,
"forks" : 3,
"mean_ops" : 525944.6622679544
"mean_ops" : 787544.107063881
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
"iterations" : 4,
"threads" : 1,
"forks" : 3,
"mean_ops" : 484353.78910261835
"mean_ops" : 594383.3501367184
}
28 changes: 28 additions & 0 deletions ranger-drove-client/pom.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2024 Authors, Flipkart Internet Pvt. Ltd.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
Expand Down Expand Up @@ -41,6 +57,18 @@
<scope>test</scope>
<type>test-jar</type>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>${logback.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>${mockito.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright 2015 Flipkart Internet Pvt. Ltd.
* <p>
* Copyright 2024 Authors, Flipkart Internet Pvt. Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
*
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -25,7 +25,7 @@
import io.appform.ranger.drove.serde.DroveResponseDataDeserializer;
import io.appform.ranger.drove.servicefinderhub.DroveServiceDataSource;
import io.appform.ranger.drove.servicefinderhub.DroveServiceFinderHubBuilder;
import io.appform.ranger.drove.utils.DroveCommunicator;
import io.appform.ranger.drove.common.DroveCommunicator;
import lombok.Builder;
import lombok.Getter;
import lombok.experimental.SuperBuilder;
Expand All @@ -38,7 +38,7 @@ public abstract class AbstractRangerDroveHubClient<T, R extends ServiceRegistry<
extends AbstractRangerHubClient<T, R, D> {

private final DroveUpstreamConfig clientConfig;
private final DroveCommunicator<T> droveCommunicator;
private final DroveCommunicator droveCommunicator;

@Builder.Default
private final ServiceNodeSelector<T> nodeSelector = new RandomServiceNodeSelector<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright 2015 Flipkart Internet Pvt. Ltd.
* <p>
* Copyright 2024 Authors, Flipkart Internet Pvt. Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
*
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright 2015 Flipkart Internet Pvt. Ltd.
* <p>
* Copyright 2024 Authors, Flipkart Internet Pvt. Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
*
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright 2015 Flipkart Internet Pvt. Ltd.
* <p>
* Copyright 2024 Authors, Flipkart Internet Pvt. Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
*
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* Copyright 2015 Flipkart Internet Pvt. Ltd.
* <p>
* Copyright 2024 Authors, Flipkart Internet Pvt. Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
*
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -26,6 +26,7 @@
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import lombok.val;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.RegisterExtension;

Expand All @@ -50,11 +51,11 @@ public abstract class BaseRangerDroveClientTest {

@BeforeEach
public void prepareHttpMocks() throws Exception {
wireMockExtension.stubFor(get(urlEqualTo("/apis/v1/endpoints/app/TEST_APP"))
wireMockExtension.stubFor(get(urlPathEqualTo("/apis/v1/endpoints"))
.willReturn(aResponse()
.withBody(objectMapper.writeValueAsBytes(
ApiResponse.success(List.of(new ExposedAppInfo(
"test",
"TEST_APP",
"test-0.1",
"test.appform.io",
Map.of(),
Expand All @@ -63,11 +64,6 @@ public void prepareHttpMocks() throws Exception {
32456,
PortType.HTTP)))))))
.withStatus(200)));
wireMockExtension.stubFor(get(urlEqualTo("/apis/v1/endpoints/app/OTHER_APP"))
.willReturn(aResponse()
.withBody(objectMapper.writeValueAsBytes(
ApiResponse.success(List.of())))
.withStatus(200)));

val response = ApiResponse.success(Map.of(
"TEST_APP-1",
Expand Down Expand Up @@ -120,6 +116,11 @@ public void prepareHttpMocks() throws Exception {
clientConfig = DroveUpstreamConfig.builder()
.endpoints(List.of("http://localhost:" + wireMockExtension.getPort()))
.build();
log.debug("Started http subsystem");
log.debug("Started http subsystem. Wiremock port: {}", wireMockExtension.getPort());
}

@AfterAll
public static void shutdown() {
wireMockExtension.shutdownServer();
}
}
Loading

0 comments on commit d995b55

Please sign in to comment.