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

refactoring registration and provider service to make it resilient for conflict errors #200

Merged
merged 4 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
package com.adobe.aio.event.management;

import com.adobe.aio.event.management.feign.FeignRegistrationService;
import com.adobe.aio.event.management.model.Registration;
import com.adobe.aio.event.management.model.RegistrationCreateModel;
import com.adobe.aio.event.management.model.RegistrationPaginatedModel;
import com.adobe.aio.event.management.model.RegistrationUpdateModel;
import com.adobe.aio.workspace.Workspace;
import com.adobe.aio.event.management.model.Registration;
import com.fasterxml.jackson.core.JsonProcessingException;
import java.util.List;
import java.util.Optional;

Expand All @@ -27,6 +26,8 @@ public interface RegistrationService {

void delete(String registrationId);

Optional<Registration> createOrUpdateRegistration(RegistrationCreateModel.Builder registrationCreateModelBuilder);

Optional<Registration> createRegistration(RegistrationCreateModel.Builder registrationCreateModelBuilder);

Optional<Registration> updateRegistration(String registrationId, RegistrationUpdateModel.Builder registrationUpdateModelBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@

import feign.FeignException;
import feign.Response;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Optional;

public class ConflictException extends FeignException {

private final Logger logger = LoggerFactory.getLogger(this.getClass());
private final String conflictingId;

public ConflictException(Response response, FeignException exception) {
super(response.status(), exception.getMessage(), response.request(), exception);
Optional<String> conflictingIdOptional = response.headers().get("x-conflicting-id").stream().findFirst();
conflictingId = conflictingIdOptional.isPresent() ? conflictingIdOptional.get() : null;
}

public String getConflictingId() {
return conflictingId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,11 @@ public Optional<Provider> createProvider(final ProviderInputModel providerInputM
public Optional<Provider> createOrUpdateProvider(final ProviderInputModel providerInputModel) {
try {
return createProvider(providerInputModel);
}
catch (ConflictException e){
logger.info("Another provider (providerMetadata: `{}`, imsOrg:`{}`, instanceId: `{}` )"
+ " exist with conflicting natural keys, trying to update it ...",
providerInputModel.getProviderMetadataId(), workspace.getImsOrgId(),
providerInputModel.getInstanceId());
String providerId = this.findProviderBy(providerInputModel.getProviderMetadataId(),
providerInputModel.getInstanceId())
.orElseThrow(() -> new AIOException("Race condition error: the provider "
+ "(`" + providerInputModel.getProviderMetadataId() + "`,"
+ "`" + workspace.getImsOrgId() + "`,"
+ "`" + providerInputModel.getInstanceId()+ "`)"
+ " may have been deleted just after a Conflict `" + e.getMessage()
+ "` was detected while creating it"))
.getId();
return updateProvider(providerId,providerInputModel);
} catch (ConflictException e){
String conflictingProviderId = e.getConflictingId();
logger.info("Another provider (id: `{}` ) exist with conflict due to {}, trying to update it ...",
conflictingProviderId, e.getMessage());
return updateProvider(conflictingProviderId, providerInputModel);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@
*/
package com.adobe.aio.event.management.feign;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import org.apache.commons.lang3.StringUtils;
import static com.adobe.aio.util.Constants.API_MANAGEMENT_URL;

import com.adobe.aio.event.management.RegistrationService;
import com.adobe.aio.event.management.api.RegistrationApi;
Expand All @@ -29,11 +25,17 @@
import com.adobe.aio.util.feign.FeignUtil;
import com.adobe.aio.workspace.Workspace;
import feign.RequestInterceptor;

import static com.adobe.aio.util.Constants.*;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class FeignRegistrationService implements RegistrationService {

private final Logger logger = LoggerFactory.getLogger(this.getClass());

private final RegistrationApi registrationApi;
private final Workspace workspace;

Expand All @@ -48,6 +50,7 @@ public FeignRegistrationService(final Workspace workspace,
this.registrationApi = FeignUtil.getDefaultBuilder()
.requestInterceptor(authInterceptor)
.requestInterceptor(AIOHeaderInterceptor.builder().workspace(workspace).build())
.errorDecoder(new ConflictErrorDecoder())
.target(RegistrationApi.class, apiUrl);
this.workspace = workspace;
}
Expand All @@ -70,6 +73,18 @@ public void delete(String registrationId) {
workspace.getWorkspaceId(), registrationId);
}

@Override
public Optional<Registration> createOrUpdateRegistration(
RegistrationCreateModel.Builder registrationCreateModelBuilder) {
try {
return createRegistration(registrationCreateModelBuilder);
} catch (ConflictException ex) {
String conflictingRegistrationId = ex.getConflictingId();
logger.info("Another registration (id `{}`) exists with conflict due to {}. Trying to update it...", conflictingRegistrationId, ex.getMessage());
return updateRegistration(conflictingRegistrationId, registrationCreateModelBuilder);
}
}

@Override
public Optional<Registration> createRegistration(
RegistrationCreateModel.Builder registrationCreateModelBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.net.URL;
import java.util.Optional;
import java.util.Set;
import java.util.function.Supplier;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -55,33 +56,40 @@ public static EventsOfInterestInputModel.Builder getTestEventsOfInterestBuilder(
.providerId(providerId);
}

public Registration createJournalRegistration(String registrationName,
public Registration createOrUpdateJournalRegistration(String registrationName,
String providerId, String eventCode){
return createRegistration(RegistrationCreateModel.builder()
return createOrUpdateRegistration(RegistrationCreateModel.builder()
.name(registrationName)
.description(TEST_DESCRIPTION)
.deliveryType(DELIVERY_TYPE_JOURNAL)
.addEventsOfInterests(getTestEventsOfInterestBuilder(providerId, eventCode).build()));
}

public Registration createRuntimeWebhookRegistration(String registrationName, String providerId,
public Registration createOrUpdateRuntimeWebhookRegistration(String registrationName, String providerId,
String eventCode, String runtimeAction) {
return createRegistration(RegistrationCreateModel.builder()
return createOrUpdateRegistration(RegistrationCreateModel.builder()
.name(registrationName)
.description(TEST_DESCRIPTION)
.deliveryType(DELIVERY_TYPE_WEBHOOK)
.runtimeAction(runtimeAction)
.addEventsOfInterests(getTestEventsOfInterestBuilder(providerId, eventCode).build()));
}

public Registration createRegistration(
public Registration createOrUpdateRegistration(
RegistrationCreateModel.Builder registrationInputModelBuilder) {
return assertRegistrationCreatedResponseWithEventsOfInterest(registrationInputModelBuilder,
() -> registrationService.createOrUpdateRegistration(registrationInputModelBuilder));
}

public Registration assertRegistrationCreatedResponseWithEventsOfInterest(
RegistrationCreateModel.Builder registrationInputModelBuilder,
Supplier<Optional<Registration>> registrationSupplier) {
RegistrationCreateModel registrationInputModel =
registrationInputModelBuilder.clientId(this.workspace.getApiKey()).build();
Optional<Registration> registration = registrationService.createRegistration(registrationInputModelBuilder);
assertTrue(registration.isPresent());
Registration registrationCreated = registration.get();
logger.info("Created AIO Event Registration: {}", registration.get());
Optional<Registration> registrationOptional = registrationSupplier.get();
assertTrue(registrationOptional.isPresent());
Registration registrationCreated = registrationOptional.get();
logger.info("Created AIO Event Registration: {}", registrationOptional.get());

assertNotNull(registrationCreated.getRegistrationId());
assertEquals(registrationInputModel.getDescription(), registrationCreated.getDescription());
Expand All @@ -95,30 +103,31 @@ public Registration createRegistration(
assertEquals(registrationInputModel.getWebhookUrl(), registrationCreated.getWebhookUrl());
}
} else {
assertNull(registration.get().getWebhookUrl());
assertUrl(registration.get().getJournalUrl().getHref());
assertNull(registrationOptional.get().getWebhookUrl());
assertUrl(registrationOptional.get().getJournalUrl().getHref());
}

Set<EventsOfInterest> eventsOfInterestSet = registration.get().getEventsOfInterests();
Set<EventsOfInterest> eventsOfInterestSet = registrationOptional.get().getEventsOfInterests();
assertEquals(registrationInputModel.getEventsOfInterests().size(),eventsOfInterestSet.size());
for(EventsOfInterestInputModel eventsOfInterestInput: registrationInputModel.getEventsOfInterests()){
assertTrue(eventsOfInterestSet.stream()
.anyMatch(eventsOfInterest -> eventsOfInterest.getEventCode()
.equals(eventsOfInterestInput.getEventCode())));
.anyMatch(eventsOfInterest -> eventsOfInterest.getEventCode()
.equals(eventsOfInterestInput.getEventCode())));
}

assertEquals("verified", registrationCreated.getWebhookStatus());
assertEquals(true, registrationCreated.isEnabled());

assertUrl(registration.get().getTraceUrl().getHref());
assertUrl(registration.get().getJournalUrl().getHref());
assertUrl(registrationOptional.get().getTraceUrl().getHref());
assertUrl(registrationOptional.get().getJournalUrl().getHref());

assertNotNull(registration.get().getCreatedDate());
assertNotNull(registration.get().getUpdatedDate());
return registration.get();
assertNotNull(registrationOptional.get().getCreatedDate());
assertNotNull(registrationOptional.get().getUpdatedDate());
return registrationOptional.get();
}

public Registration updateRegistration(Registration registrationToUpdate, String runtimeActionToUpdate) {
public Registration updateRuntimeWebhookRegistration(Registration registrationToUpdate,
String runtimeActionToUpdate) {
EventsOfInterest eventsOfInterest = registrationToUpdate.getEventsOfInterests().iterator().next();
String providerId = eventsOfInterest.getProviderId();
String eventCode = eventsOfInterest.getEventCode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.adobe.aio.event.management.model.Registration;
import com.adobe.aio.event.publish.PublishServiceTester;
import com.adobe.aio.util.WorkspaceUtil;
import java.util.UUID;
import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -67,7 +66,7 @@ public void testJournalPolling()
try {
providerId = providerServiceTester.createOrUpdateProvider(TEST_EVENT_PROVIDER_LABEL,
ProviderServiceIntegrationTest.TEST_EVENT_CODE).getId();
Registration registration = registrationServiceTester.createJournalRegistration(
Registration registration = registrationServiceTester.createOrUpdateJournalRegistration(
TEST_REGISTRATION_NAME, providerId, TEST_EVENT_CODE);
registrationId = registration.getRegistrationId();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
*/
package com.adobe.aio.event.management;

import com.adobe.aio.event.management.model.ProviderInputModel;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

Expand Down Expand Up @@ -107,34 +109,29 @@ public void createGetUpdateDelete() {
logger.info("Deleted EventMetadata {} from AIO Events Provider `{}`", TEST_EVENT_CODE,
providerById);

try {
providerService.createProvider(
getTestProviderInputModelBuilder(TEST_EVENT_PROVIDER_LABEL).instanceId(instanceId)
.build());
fail("We should have had a ConflictException thrown");
} catch (ConflictException ex) {
logger.info("Cannot create an AIO Events provider with the same instanceId: {}",
ex.getMessage());
}

// update provider and event metadata
String updatedProviderDescription = "updated Provider Description";
Optional<Provider> updatedProvider = providerService.updateProvider(providerId,
getTestProviderInputModelBuilder(TEST_EVENT_PROVIDER_LABEL)
Provider updatedProvider = createOrUpdateProvider(getTestProviderInputModelBuilder(TEST_EVENT_PROVIDER_LABEL)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here

.instanceId(instanceId)
.description(updatedProviderDescription)
.eventDeliveryFormat(DELIVERY_FORMAT_ADOBE_IO)
.build());
assertTrue(updatedProvider.isPresent());
.build(), Collections.singleton(getTestEventMetadataBuilder(TEST_EVENT_CODE).build()));
assertTrue(updatedProvider != null);
logger.info("Updated AIO Events Provider: {}", provider);
assertEquals(providerId, updatedProvider.get().getId());
assertEquals(updatedProviderDescription, updatedProvider.get().getDescription());
assertEquals(DELIVERY_FORMAT_ADOBE_IO, updatedProvider.get().getEventDeliveryFormat());
assertEquals(providerId, updatedProvider.getId());
assertEquals(updatedProviderDescription, updatedProvider.getDescription());
assertEquals(DELIVERY_FORMAT_ADOBE_IO, updatedProvider.getEventDeliveryFormat());

updatedEventMetadataDescription = "updated EventMetadata Description";
eventMetadata = providerService.updateEventMetadata(providerId,
getTestEventMetadataBuilder(TEST_EVENT_CODE).description(updatedEventMetadataDescription)
.build());

providerService.createEventMetadata(providerId,
getTestEventMetadataBuilder(TEST_EVENT_CODE).build());
assertTrue(eventMetadata.isPresent());
logger.info("Added EventMetadata `{}` to AIO Events Provider `{}`", eventMetadata,
logger.info("Updated EventMetadata `{}` of AIO Events Provider `{}`", eventMetadata,
providerId);
assertEquals(updatedEventMetadataDescription, eventMetadata.get().getDescription());

providerService.deleteAllEventMetadata(providerId);
assertTrue(providerService.getEventMetadata(providerId).isEmpty());
logger.info("Deleted All EventMetadata from AIO Events Provider `{}`", providerId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import java.util.Optional;

import java.util.function.Supplier;
import org.apache.commons.lang3.StringUtils;

import com.adobe.aio.event.management.model.Registration;
Expand Down Expand Up @@ -51,24 +52,19 @@ public void createGetDeleteJournalRegistration() {
try {
providerId = providerServiceTester.createOrUpdateProvider(TEST_EVENT_PROVIDER_LABEL,
TEST_EVENT_CODE).getId();
Registration registration = createJournalRegistration(TEST_REGISTRATION_NAME, providerId,
Registration registration = createOrUpdateJournalRegistration(TEST_REGISTRATION_NAME, providerId,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be we could test both path ?
using createOrUpdate once and then createOrUpdate again ... once created
here and may be in ProviderServiceIntegrationTest too
wdyt ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get this

Copy link
Collaborator Author

@abhupadh abhupadh Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEST_EVENT_CODE);
registrationId = registration.getRegistrationId();
Optional<Registration> found = registrationService.findById(registrationId);
assertTrue(found.isPresent());
logger.info("Found AIO Event Registration: {}", found.get());
assertEquals(registrationId, found.get().getRegistrationId());
assertEquals(registration.getClientId(), found.get().getClientId());
assertEquals(registration.getDescription(), found.get().getDescription());
assertEquals(registration.getName(), found.get().getName());
assertEquals(registration.getDeliveryType(), found.get().getDeliveryType());
assertEquals(registration.getEventsOfInterests(),
found.get().getEventsOfInterests());
assertEquals(registration.getWebhookStatus(), found.get().getWebhookStatus());
assertEquals(registration.isEnabled(), found.get().isEnabled());
assertEquals(registration.getWebhookUrl(), found.get().getWebhookUrl());
assertEquals(registration.getJournalUrl().getHref(), found.get().getJournalUrl().getHref());
assertEquals(registration.getTraceUrl().getHref(), found.get().getTraceUrl().getHref());
String finalRegistrationId = registration.getRegistrationId();
assertCreatedOrUpdatedRegistrationMatchesWithFoundRegistration(registration,
() -> registrationService.findById(finalRegistrationId));

// covering the update path
registration = createOrUpdateJournalRegistration(TEST_REGISTRATION_NAME, providerId,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add the same coverage for the ProviderService pretty please ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well,I have done this here in the ProviderServiceIntegrationTest, is there any other place I need to do that you meant?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I did not catch this, thanks !

TEST_EVENT_CODE);
String finalUpdatedRegistrationId = registration.getRegistrationId();
assertCreatedOrUpdatedRegistrationMatchesWithFoundRegistration(registration,
() -> registrationService.findById(finalUpdatedRegistrationId));
} catch (Exception e) {
logger.error(e.getMessage(), e);
fail(e.getMessage());
Expand All @@ -81,4 +77,23 @@ public void createGetDeleteJournalRegistration() {
}
}
}

public void assertCreatedOrUpdatedRegistrationMatchesWithFoundRegistration(Registration registration,
Supplier<Optional<Registration>> foundRegistrationSupplier) {
Optional<Registration> foundRegistrationOptional = foundRegistrationSupplier.get();
assertTrue(foundRegistrationOptional.isPresent());
logger.info("Found AIO Event Registration: {}", foundRegistrationOptional.get());
assertEquals(registration.getRegistrationId(), foundRegistrationOptional.get().getRegistrationId());
assertEquals(registration.getClientId(), foundRegistrationOptional.get().getClientId());
assertEquals(registration.getDescription(), foundRegistrationOptional.get().getDescription());
assertEquals(registration.getName(), foundRegistrationOptional.get().getName());
assertEquals(registration.getDeliveryType(), foundRegistrationOptional.get().getDeliveryType());
assertEquals(registration.getEventsOfInterests(),
foundRegistrationOptional.get().getEventsOfInterests());
assertEquals(registration.getWebhookStatus(), foundRegistrationOptional.get().getWebhookStatus());
assertEquals(registration.isEnabled(), foundRegistrationOptional.get().isEnabled());
assertEquals(registration.getWebhookUrl(), foundRegistrationOptional.get().getWebhookUrl());
assertEquals(registration.getJournalUrl().getHref(), foundRegistrationOptional.get().getJournalUrl().getHref());
assertEquals(registration.getTraceUrl().getHref(), foundRegistrationOptional.get().getTraceUrl().getHref());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import com.adobe.aio.event.management.ProviderServiceTester;
import com.adobe.aio.event.management.RegistrationServiceTester;
import java.util.UUID;
import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.Test;

Expand All @@ -47,7 +46,7 @@ public void publishEventTest() {
try {
providerId = providerServiceTester.createOrUpdateProvider(TEST_EVENT_PROVIDER_LABEL,
TEST_EVENT_CODE).getId();
registrationId = registrationServiceTester.createJournalRegistration(
registrationId = registrationServiceTester.createOrUpdateJournalRegistration(
TEST_REGISTRATION_NAME, providerId, TEST_EVENT_CODE).getRegistrationId();

assertNotNull(publishCloudEvent(providerId, TEST_EVENT_CODE));
Expand Down