This is an automated email from the ASF dual-hosted git repository. zehnder pushed a commit to branch 3298-add-cypress-e2e-tests-for-compact-adapter-feature-3 in repository https://gitbox.apache.org/repos/asf/streampipes.git
commit 5fdaa37501a3141d396e65c9e02fbdf9f81ea119 Author: Philipp Zehnder <[email protected]> AuthorDate: Mon Oct 21 15:21:25 2024 +0200 feat(#3298): Return status code conflict when adapter id is already taken --- .../management/AdapterMasterManagement.java | 58 ++++++++++++++-------- .../management/AdapterMasterManagementTest.java | 34 ++++++------- .../management/AdapterResourceManager.java | 12 +++-- .../management/AdapterResourceManagerTest.java | 40 +++++++++++++-- .../rest/impl/connect/CompactAdapterResource.java | 27 +++++++--- .../support/utils/connect/CompactAdapterUtils.ts | 13 ++++- ui/cypress/support/utils/connect/ConnectUtils.ts | 5 ++ .../tests/connect/compact/addCompactAdapter.ts | 24 +++++++++ 8 files changed, 161 insertions(+), 52 deletions(-) diff --git a/streampipes-connect-management/src/main/java/org/apache/streampipes/connect/management/management/AdapterMasterManagement.java b/streampipes-connect-management/src/main/java/org/apache/streampipes/connect/management/management/AdapterMasterManagement.java index 62c514b1e6..5c42478431 100644 --- a/streampipes-connect-management/src/main/java/org/apache/streampipes/connect/management/management/AdapterMasterManagement.java +++ b/streampipes-connect-management/src/main/java/org/apache/streampipes/connect/management/management/AdapterMasterManagement.java @@ -53,10 +53,11 @@ public class AdapterMasterManagement { private final DataStreamResourceManager dataStreamResourceManager; - public AdapterMasterManagement(IAdapterStorage adapterInstanceStorage, - AdapterResourceManager adapterResourceManager, - DataStreamResourceManager dataStreamResourceManager, - AdapterMetrics adapterMetrics + public AdapterMasterManagement( + IAdapterStorage adapterInstanceStorage, + AdapterResourceManager adapterResourceManager, + DataStreamResourceManager dataStreamResourceManager, + AdapterMetrics adapterMetrics ) { this.adapterInstanceStorage = adapterInstanceStorage; this.adapterMetrics = adapterMetrics; @@ -64,28 +65,40 @@ public class AdapterMasterManagement { this.dataStreamResourceManager = dataStreamResourceManager; } - public void addAdapter(AdapterDescription ad, - String adapterElementId, - String principalSid) + public void addAdapter( + AdapterDescription adapterDescription, + String adapterId, + String principalSid + ) throws AdapterException { - // Create elementId for adapter + // Create elementId for datastream var dataStreamElementId = ElementIdGenerator.makeElementId(SpDataStream.class); - ad.setElementId(adapterElementId); - ad.setCreatedAt(System.currentTimeMillis()); - ad.setCorrespondingDataStreamElementId(dataStreamElementId); + adapterDescription.setElementId(adapterId); + adapterDescription.setCreatedAt(System.currentTimeMillis()); + adapterDescription.setCorrespondingDataStreamElementId(dataStreamElementId); // Add EventGrounding to AdapterDescription var eventGrounding = GroundingUtils.createEventGrounding(); - ad.setEventGrounding(eventGrounding); + adapterDescription.setEventGrounding(eventGrounding); - this.adapterResourceManager.encryptAndCreate(ad); + this.adapterResourceManager.encryptAndCreate(adapterDescription); - // Create stream - var storedDescription = new SourcesManagement().createAdapterDataStream(ad, dataStreamElementId); - storedDescription.setCorrespondingAdapterId(adapterElementId); + // Stream is only created if the adpater is successfully stored + createDataStreamForAdapter(adapterDescription, adapterId, dataStreamElementId, principalSid); + } + + private void createDataStreamForAdapter( + AdapterDescription adapterDescription, + String adapterId, + String streamId, + String principalSid + ) throws AdapterException { + var storedDescription = new SourcesManagement() + .createAdapterDataStream(adapterDescription, streamId); + storedDescription.setCorrespondingAdapterId(adapterId); installDataSource(storedDescription, principalSid, true); - LOG.info("Install source (source URL: {} in backend", ad.getElementId()); + LOG.info("Install source (source URL: {} in backend", adapterDescription.getElementId()); } public AdapterDescription getAdapter(String elementId) throws AdapterException { @@ -163,7 +176,8 @@ public class AdapterMasterManagement { var baseUrl = new ExtensionsServiceEndpointGenerator().getEndpointBaseUrl( ad.getAppId(), SpServiceUrlProvider.ADAPTER, - ad.getDeploymentConfiguration().getDesiredServiceTags() + ad.getDeploymentConfiguration() + .getDesiredServiceTags() ); // Update selected endpoint URL of adapter @@ -182,9 +196,11 @@ public class AdapterMasterManagement { } } - private void installDataSource(SpDataStream stream, - String principalSid, - boolean publicElement) throws AdapterException { + private void installDataSource( + SpDataStream stream, + String principalSid, + boolean publicElement + ) throws AdapterException { try { new DataStreamVerifier(stream).verifyAndAdd(principalSid, publicElement); } catch (SepaParseException e) { diff --git a/streampipes-connect-management/src/test/java/org/apache/streampipes/connect/management/management/AdapterMasterManagementTest.java b/streampipes-connect-management/src/test/java/org/apache/streampipes/connect/management/management/AdapterMasterManagementTest.java index 6d920fb751..e1e18324ce 100644 --- a/streampipes-connect-management/src/test/java/org/apache/streampipes/connect/management/management/AdapterMasterManagementTest.java +++ b/streampipes-connect-management/src/test/java/org/apache/streampipes/connect/management/management/AdapterMasterManagementTest.java @@ -36,12 +36,12 @@ import static org.mockito.Mockito.when; public class AdapterMasterManagementTest { @Test - public void getAdapterFailNull() { - AdapterInstanceStorageImpl adapterStorage = mock(AdapterInstanceStorageImpl.class); - AdapterResourceManager resourceManager = mock(AdapterResourceManager.class); + public void getAdapter_FailNull() { + var adapterStorage = mock(AdapterInstanceStorageImpl.class); + var resourceManager = mock(AdapterResourceManager.class); when(adapterStorage.findAll()).thenReturn(null); - AdapterMasterManagement adapterMasterManagement = + var adapterMasterManagement = new AdapterMasterManagement( adapterStorage, resourceManager, @@ -53,13 +53,13 @@ public class AdapterMasterManagementTest { } @Test - public void getAdapterFail() { - List<AdapterDescription> adapterDescriptions = List.of(new AdapterDescription()); - AdapterInstanceStorageImpl adapterStorage = mock(AdapterInstanceStorageImpl.class); - AdapterResourceManager resourceManager = mock(AdapterResourceManager.class); + public void getAdapter_Fail() { + var adapterDescriptions = List.of(new AdapterDescription()); + var adapterStorage = mock(AdapterInstanceStorageImpl.class); + var resourceManager = mock(AdapterResourceManager.class); when(adapterStorage.findAll()).thenReturn(adapterDescriptions); - AdapterMasterManagement adapterMasterManagement = + var adapterMasterManagement = new AdapterMasterManagement( adapterStorage, resourceManager, @@ -71,10 +71,10 @@ public class AdapterMasterManagementTest { } @Test - public void getAllAdaptersSuccess() throws AdapterException { - List<AdapterDescription> adapterDescriptions = List.of(new AdapterDescription()); - AdapterInstanceStorageImpl adapterStorage = mock(AdapterInstanceStorageImpl.class); - AdapterResourceManager resourceManager = mock(AdapterResourceManager.class); + public void getAllAdapters_Success() throws AdapterException { + var adapterDescriptions = List.of(new AdapterDescription()); + var adapterStorage = mock(AdapterInstanceStorageImpl.class); + var resourceManager = mock(AdapterResourceManager.class); when(adapterStorage.findAll()).thenReturn(adapterDescriptions); AdapterMasterManagement adapterMasterManagement = @@ -91,12 +91,12 @@ public class AdapterMasterManagementTest { } @Test - public void getAllAdaptersFail() { - AdapterInstanceStorageImpl adapterStorage = mock(AdapterInstanceStorageImpl.class); - AdapterResourceManager resourceManager = mock(AdapterResourceManager.class); + public void getAllAdapters_Fail() { + var adapterStorage = mock(AdapterInstanceStorageImpl.class); + var resourceManager = mock(AdapterResourceManager.class); when(adapterStorage.findAll()).thenReturn(null); - AdapterMasterManagement adapterMasterManagement = + var adapterMasterManagement = new AdapterMasterManagement( adapterStorage, resourceManager, diff --git a/streampipes-resource-management/src/main/java/org/apache/streampipes/resource/management/AdapterResourceManager.java b/streampipes-resource-management/src/main/java/org/apache/streampipes/resource/management/AdapterResourceManager.java index 72c77121aa..f477636033 100644 --- a/streampipes-resource-management/src/main/java/org/apache/streampipes/resource/management/AdapterResourceManager.java +++ b/streampipes-resource-management/src/main/java/org/apache/streampipes/resource/management/AdapterResourceManager.java @@ -17,6 +17,7 @@ */ package org.apache.streampipes.resource.management; +import org.apache.streampipes.commons.exceptions.connect.AdapterException; import org.apache.streampipes.model.connect.adapter.AdapterDescription; import org.apache.streampipes.model.util.Cloner; import org.apache.streampipes.resource.management.secret.SecretProvider; @@ -39,10 +40,15 @@ public class AdapterResourceManager extends AbstractResourceManager<IAdapterStor * @param adapterDescription input adapter description * @return the id of the created adapter */ - public String encryptAndCreate(AdapterDescription adapterDescription) { - AdapterDescription encryptedAdapterDescription = cloneAndEncrypt(adapterDescription); + public String encryptAndCreate(AdapterDescription adapterDescription) throws AdapterException { + var encryptedAdapterDescription = cloneAndEncrypt(adapterDescription); encryptedAdapterDescription.setRev(null); - return db.persist(encryptedAdapterDescription).v; + + try { + return db.persist(encryptedAdapterDescription).v; + } catch (org.lightcouch.DocumentConflictException e) { + throw new AdapterException("Conflict occurred while creating the adapter", e); + } } /** diff --git a/streampipes-resource-management/src/test/java/org/apache/streampipes/resource/management/AdapterResourceManagerTest.java b/streampipes-resource-management/src/test/java/org/apache/streampipes/resource/management/AdapterResourceManagerTest.java index d815855490..21888492cf 100644 --- a/streampipes-resource-management/src/test/java/org/apache/streampipes/resource/management/AdapterResourceManagerTest.java +++ b/streampipes-resource-management/src/test/java/org/apache/streampipes/resource/management/AdapterResourceManagerTest.java @@ -18,25 +18,59 @@ package org.apache.streampipes.resource.management; +import org.apache.streampipes.commons.exceptions.connect.AdapterException; +import org.apache.streampipes.model.Tuple2; import org.apache.streampipes.model.connect.adapter.AdapterDescription; import org.apache.streampipes.storage.api.IAdapterStorage; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class AdapterResourceManagerTest { + private IAdapterStorage storage; + private AdapterResourceManager adapterResourceManager; + + @BeforeEach + void setUp() { + storage = mock(IAdapterStorage.class); + adapterResourceManager = new AdapterResourceManager(storage); + } + @Test - public void encryptAndUpdateValidateDescriptionStored() { + public void encryptAndUpdate_ValidateDescriptionStored() { - IAdapterStorage storage = mock(IAdapterStorage.class); - AdapterResourceManager adapterResourceManager = new AdapterResourceManager(storage); adapterResourceManager.encryptAndUpdate(new AdapterDescription()); verify(storage, times(1)).updateElement(any()); } + + @Test + void encryptAndCreate_ReturnsIdOnSuccess() throws AdapterException { + var id = "adapterId"; + + when(storage.persist(any())).thenReturn(new Tuple2<>(true, id)); + var result = adapterResourceManager.encryptAndCreate(new AdapterDescription()); + + verify(storage, times(1)).persist(any()); + assertEquals(id, result); + } + + @Test + void encryptAndCreate_ThrowsAdapterExceptionOnConflict() { + + doThrow(new org.lightcouch.DocumentConflictException("Conflict")).when(storage).persist(any()); + + assertThrows(AdapterException.class, () -> adapterResourceManager.encryptAndCreate(new AdapterDescription())); + } + } diff --git a/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/connect/CompactAdapterResource.java b/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/connect/CompactAdapterResource.java index 88424c2256..4066f35b96 100644 --- a/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/connect/CompactAdapterResource.java +++ b/streampipes-rest/src/main/java/org/apache/streampipes/rest/impl/connect/CompactAdapterResource.java @@ -37,6 +37,7 @@ import org.apache.streampipes.storage.management.StorageDispatcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; @@ -58,7 +59,7 @@ public class CompactAdapterResource extends AbstractAdapterResource<AdapterMaste public CompactAdapterResource() { super(() -> new AdapterMasterManagement( StorageDispatcher.INSTANCE.getNoSqlStore() - .getAdapterInstanceStorage(), + .getAdapterInstanceStorage(), new SpResourceManager().manageAdapters(), new SpResourceManager().manageDataStreams(), AdapterMetricsManager.INSTANCE.getAdapterMetrics() @@ -82,11 +83,22 @@ public class CompactAdapterResource extends AbstractAdapterResource<AdapterMaste var adapterDescription = getGeneratedAdapterDescription(compactAdapter); var principalSid = getAuthenticatedUserSid(); + var adapterId = adapterDescription.getElementId(); + try { - var adapterId = adapterDescription.getElementId(); managementService.addAdapter(adapterDescription, adapterId, principalSid); + } catch (AdapterException e) { + LOG.error("Error while storing the adapterDescription with appId {}. An adapter with the given id already exists.", + adapterDescription.getAppId(), e + ); + return ResponseEntity.status(HttpStatus.CONFLICT) + .body(Notifications.error(e.getMessage())); + } + + try { if (compactAdapter.createOptions() != null) { - if (compactAdapter.createOptions().persist()) { + if (compactAdapter.createOptions() + .persist()) { var storedAdapter = managementService.getAdapter(adapterId); var status = new PersistPipelineHandler( getNoSqlStorage().getPipelineTemplateStorage(), @@ -96,7 +108,8 @@ public class CompactAdapterResource extends AbstractAdapterResource<AdapterMaste getAuthenticatedUserSid() ).createAndStartPersistPipeline(storedAdapter); } - if (compactAdapter.createOptions().start()) { + if (compactAdapter.createOptions() + .start()) { managementService.startStreamAdapter(adapterId); } } @@ -143,8 +156,10 @@ public class CompactAdapterResource extends AbstractAdapterResource<AdapterMaste return new CompactAdapterManagement(generators).convertToAdapterDescription(compactAdapter); } - private AdapterDescription getGeneratedAdapterDescription(CompactAdapter compactAdapter, - AdapterDescription existingAdapter) throws Exception { + private AdapterDescription getGeneratedAdapterDescription( + CompactAdapter compactAdapter, + AdapterDescription existingAdapter + ) throws Exception { var generators = adapterGenerationSteps.getGenerators(); return new CompactAdapterManagement(generators).convertToAdapterDescription(compactAdapter, existingAdapter); } diff --git a/ui/cypress/support/utils/connect/CompactAdapterUtils.ts b/ui/cypress/support/utils/connect/CompactAdapterUtils.ts index db0c4ab444..d6d059950e 100644 --- a/ui/cypress/support/utils/connect/CompactAdapterUtils.ts +++ b/ui/cypress/support/utils/connect/CompactAdapterUtils.ts @@ -19,10 +19,14 @@ import { CompactAdapter } from '../../../../projects/streampipes/platform-services/src/lib/model/gen/streampipes-model'; export class CompactAdapterUtils { - public static storeCompactAdapter(compactAdapter: CompactAdapter) { + public static storeCompactAdapter( + compactAdapter: CompactAdapter, + failOnStatusCode = true, + ) { return this.postCompactAdapterRequest( 'application/json', compactAdapter, + failOnStatusCode, ); } @@ -30,12 +34,17 @@ export class CompactAdapterUtils { return this.postCompactAdapterRequest('application/yml', body); } - private static postCompactAdapterRequest(contentType: string, body: any) { + private static postCompactAdapterRequest( + contentType: string, + body: any, + failOnStatusCode = true, + ) { const token = window.localStorage.getItem('auth-token'); return cy.request({ method: 'POST', url: '/streampipes-backend/api/v2/connect/compact-adapters', body: body, + failOnStatusCode: failOnStatusCode, headers: { 'Accept': contentType, 'Content-Type': contentType, diff --git a/ui/cypress/support/utils/connect/ConnectUtils.ts b/ui/cypress/support/utils/connect/ConnectUtils.ts index 660f54fa88..eec016bd61 100644 --- a/ui/cypress/support/utils/connect/ConnectUtils.ts +++ b/ui/cypress/support/utils/connect/ConnectUtils.ts @@ -417,4 +417,9 @@ export class ConnectUtils { ConnectBtns.startAdapter().should('have.length', 1); ConnectBtns.stopAdapter().should('have.length', 0); } + + public static checkAmountOfAdapters(amount: number) { + ConnectUtils.goToConnect(); + ConnectBtns.deleteAdapter().should('have.length', amount); + } } diff --git a/ui/cypress/tests/connect/compact/addCompactAdapter.ts b/ui/cypress/tests/connect/compact/addCompactAdapter.ts index 94feb782db..fe1c38f891 100644 --- a/ui/cypress/tests/connect/compact/addCompactAdapter.ts +++ b/ui/cypress/tests/connect/compact/addCompactAdapter.ts @@ -73,6 +73,30 @@ describe('Add Compact Adapters', () => { }); }); + it('Ensure correct error code when adapter with the same id already exists', () => { + const compactAdapter = getBasicCompactMachineDataSimulator() + .setStart() + .setPersist() + .build(); + + CompactAdapterUtils.storeCompactAdapter(compactAdapter).then( + response => { + expect(response.status).to.equal(200); + ConnectUtils.checkAmountOfAdapters(1); + + // Store the same adapter a second time and validate that resource returns status of conflict + CompactAdapterUtils.storeCompactAdapter( + compactAdapter, + false, + ).then(response => { + expect(response.status).to.equal(409); + + ConnectUtils.checkAmountOfAdapters(1); + }); + }, + ); + }); + const getBasicCompactMachineDataSimulator = () => { return CompactAdapterBuilder.create( 'org.apache.streampipes.connect.iiot.adapters.simulator.machine',
