cleanup: remove unused pre-methods from the ContractNegotiationListen…

…er (#1659) * cleanup: remove unused pre methods (Events) from the ContractNegotiationListener * fix: fix codestyle and add inlining for 'update' method * refactor: inline update method and clean unused import
eclipse-edc · Jul 22, 2022 · 56a8d08 · 56a8d08
1 parent 56a6ac8
commit 56a8d08
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 165 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -86,6 +86,7 @@ in the detailed section referring to by linking pull requests or issues.
   and `:extensions:transfer-functions:transfer-functions-core` (#1482)
 * Remove `ConnectorVersionProvider`, provide version as static string (#1470)
 * Remove `samples/other/run-from-junit` (#1456)
+* Removed unused pre-methods in ContractNegotiationListener (#1649)
 
 #### Fixed
 

diff --git a/...g/eclipse/dataspaceconnector/contract/negotiation/AbstractContractNegotiationManager.java b/...g/eclipse/dataspaceconnector/contract/negotiation/AbstractContractNegotiationManager.java
@@ -18,7 +18,6 @@
 import org.eclipse.dataspaceconnector.spi.command.CommandProcessor;
 import org.eclipse.dataspaceconnector.spi.command.CommandQueue;
 import org.eclipse.dataspaceconnector.spi.command.CommandRunner;
-import org.eclipse.dataspaceconnector.spi.contract.negotiation.observe.ContractNegotiationListener;
 import org.eclipse.dataspaceconnector.spi.contract.negotiation.observe.ContractNegotiationObservable;
 import org.eclipse.dataspaceconnector.spi.contract.negotiation.store.ContractNegotiationStore;
 import org.eclipse.dataspaceconnector.spi.contract.validation.ContractValidationService;
@@ -165,11 +164,6 @@ public T build() {
         }
     }
 
-    protected void update(ContractNegotiation negotiation, Consumer<ContractNegotiationListener> observe) {
-        observable.invokeForEach(observe);
-        negotiationStore.save(negotiation);
-    }
-
     protected void breakLease(ContractNegotiation negotiation) {
         negotiationStore.save(negotiation);
     }
@@ -209,7 +203,7 @@ public BiConsumer<Object, Throwable> build() {
                             negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
                 } else if (sendRetryManager.retriesExhausted(negotiation)) {
                     negotiation.transitionError("Retry limited exceeded: " + throwable.getMessage());
-                    update(negotiation, l -> l.preError(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.failed(negotiation));
                     monitor.severe(format("[%s] attempt #%d failed to %s. Retry limit exceeded, ContractNegotiation %s moves to ERROR state",
                             getName(), negotiation.getStateCount(), operationDescription, negotiation.getId()), throwable);

diff --git a/...lipse/dataspaceconnector/contract/negotiation/ConsumerContractNegotiationManagerImpl.java b/...lipse/dataspaceconnector/contract/negotiation/ConsumerContractNegotiationManagerImpl.java
@@ -105,7 +105,7 @@ public StatusResult<ContractNegotiation> initiate(ContractOfferRequest contractO
 
         negotiation.addContractOffer(contractOffer.getContractOffer());
         negotiation.transitionInitial();
-        update(negotiation, l -> l.preRequesting(negotiation));
+        negotiationStore.save(negotiation);
         observable.invokeForEach(l -> l.initiated(negotiation));
 
         monitor.debug(String.format("[Consumer] ContractNegotiation initiated. %s is now in state %s.",
@@ -147,12 +147,12 @@ public StatusResult<ContractNegotiation> offerReceived(ClaimToken token, String
             monitor.debug("[Consumer] Contract offer received. Will be rejected: " + result.getFailureDetail());
             negotiation.setErrorDetail(result.getFailureMessages().get(0));
             negotiation.transitionDeclining();
-            update(negotiation, l -> l.preDeclining(negotiation));
+            negotiationStore.save(negotiation);
         } else {
             // Offer has been approved.
             monitor.debug("[Consumer] Contract offer received. Will be approved.");
             negotiation.transitionApproving();
-            update(negotiation, l -> l.preConsumerApproving(negotiation));
+            negotiationStore.save(negotiation);
         }
 
         monitor.debug(String.format("[Consumer] ContractNegotiation %s is now in state %s.",
@@ -195,7 +195,7 @@ public StatusResult<ContractNegotiation> confirmed(ClaimToken token, String nego
             monitor.debug("[Consumer] Contract agreement received. Validation failed.");
             negotiation.setErrorDetail("Contract rejected."); //TODO set error detail
             negotiation.transitionDeclining();
-            update(negotiation, l -> l.preDeclining(negotiation));
+            negotiationStore.save(negotiation);
             monitor.debug(String.format("[Consumer] ContractNegotiation %s is now in state %s.",
                     negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
             return StatusResult.success(negotiation);
@@ -208,7 +208,7 @@ public StatusResult<ContractNegotiation> confirmed(ClaimToken token, String nego
             // TODO: otherwise will fail. But should do it, since it's already confirmed? A duplicated message received shouldn't be an issue
             negotiation.transitionConfirmed();
         }
-        update(negotiation, l -> l.preConfirmed(negotiation));
+        negotiationStore.save(negotiation);
         observable.invokeForEach(l -> l.confirmed(negotiation));
         monitor.debug(String.format("[Consumer] ContractNegotiation %s is now in state %s.",
                 negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
@@ -235,7 +235,7 @@ public StatusResult<ContractNegotiation> declined(ClaimToken token, String negot
 
         monitor.debug("[Consumer] Contract rejection received. Abort negotiation process.");
         negotiation.transitionDeclined();
-        update(negotiation, l -> l.preDeclined(negotiation));
+        negotiationStore.save(negotiation);
         observable.invokeForEach(l -> l.declined(negotiation));
         monitor.debug(String.format("[Consumer] ContractNegotiation %s is now in state %s.",
                 negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
@@ -423,12 +423,12 @@ private BiConsumer<Object, Throwable> onInitialOfferSent(String id) {
         return new AsyncSendResultHandler(id, "send initial offer")
                 .onSuccess(negotiation -> {
                     negotiation.transitionRequested();
-                    update(negotiation, l -> l.preRequested(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.requested(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionRequesting();
-                    update(negotiation, l -> l.preRequesting(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }
@@ -438,12 +438,12 @@ private BiConsumer<Object, Throwable> onCounterOfferSent(String negotiationId) {
         return new AsyncSendResultHandler(negotiationId, "send counter offer")
                 .onSuccess(negotiation -> {
                     negotiation.transitionOffered();
-                    update(negotiation, l -> l.preConsumerOffered(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.offered(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionOffering();
-                    update(negotiation, l -> l.preConsumerOffering(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }
@@ -453,12 +453,12 @@ private BiConsumer<Object, Throwable> onAgreementSent(String negotiationId) {
         return new AsyncSendResultHandler(negotiationId, "send agreement")
                 .onSuccess(negotiation -> {
                     negotiation.transitionApproved();
-                    update(negotiation, l -> l.preConsumerApproved(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.approved(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionApproving();
-                    update(negotiation, l -> l.preConsumerApproving(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }
@@ -468,12 +468,12 @@ private BiConsumer<Object, Throwable> onRejectionSent(String negotiationId) {
         return new AsyncSendResultHandler(negotiationId, "send rejection")
                 .onSuccess(negotiation -> {
                     negotiation.transitionDeclined();
-                    update(negotiation, l -> l.preDeclined(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.declined(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionDeclining();
-                    update(negotiation, l -> l.preDeclining(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }

diff --git a/...lipse/dataspaceconnector/contract/negotiation/ProviderContractNegotiationManagerImpl.java b/...lipse/dataspaceconnector/contract/negotiation/ProviderContractNegotiationManagerImpl.java
@@ -104,7 +104,7 @@ public StatusResult<ContractNegotiation> declined(ClaimToken token, String corre
             negotiation.setContractAgreement(null);
         }
         negotiation.transitionDeclined();
-        update(negotiation, l -> l.preDeclined(negotiation));
+        negotiationStore.save(negotiation);
         observable.invokeForEach(l -> l.declined(negotiation));
         monitor.debug(String.format("[Provider] ContractNegotiation %s is now in state %s.",
                 negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
@@ -139,7 +139,7 @@ public StatusResult<ContractNegotiation> requested(ClaimToken token, ContractOff
                 .build();
 
         negotiation.transitionRequested();
-        update(negotiation, l -> l.preRequested(negotiation));
+        negotiationStore.save(negotiation);
         observable.invokeForEach(l -> l.requested(negotiation));
 
         monitor.debug(String.format("[Provider] ContractNegotiation initiated. %s is now in state %s.",
@@ -188,7 +188,7 @@ public StatusResult<ContractNegotiation> consumerApproved(ClaimToken token, Stri
 
         monitor.debug("[Provider] Contract offer has been approved by consumer.");
         negotiation.transitionConfirming();
-        update(negotiation, l -> l.preConfirming(negotiation));
+        negotiationStore.save(negotiation);
         monitor.debug(String.format("[Provider] ContractNegotiation %s is now in state %s.",
                 negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
         return StatusResult.success(negotiation);
@@ -232,7 +232,7 @@ private StatusResult<ContractNegotiation> processIncomingOffer(ContractNegotiati
             monitor.debug("[Provider] Contract offer received. Will be rejected: " + result.getFailureDetail());
             negotiation.setErrorDetail(result.getFailureMessages().get(0));
             negotiation.transitionDeclining();
-            update(negotiation, l -> l.preDeclining(negotiation));
+            negotiationStore.save(negotiation);
 
             monitor.debug(String.format("[Provider] ContractNegotiation %s is now in state %s.",
                     negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
@@ -242,7 +242,7 @@ private StatusResult<ContractNegotiation> processIncomingOffer(ContractNegotiati
         monitor.debug("[Provider] Contract offer received. Will be approved.");
         // negotiation.addContractOffer(result.getValidatedOffer()); TODO
         negotiation.transitionConfirming();
-        update(negotiation, l -> l.preConfirming(negotiation));
+        negotiationStore.save(negotiation);
         monitor.debug(String.format("[Provider] ContractNegotiation %s is now in state %s.",
                 negotiation.getId(), ContractNegotiationStates.from(negotiation.getState())));
 
@@ -385,12 +385,12 @@ private BiConsumer<Object, Throwable> onCounterOfferSent(String negotiationId) {
         return new AsyncSendResultHandler(negotiationId, "send counter offer")
                 .onSuccess(negotiation -> {
                     negotiation.transitionOffered();
-                    update(negotiation, l -> l.preProviderOffered(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.offered(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionOffering();
-                    update(negotiation, l -> l.preProviderOffering(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }
@@ -400,12 +400,12 @@ private BiConsumer<Object, Throwable> onRejectionSent(String negotiationId) {
         return new AsyncSendResultHandler(negotiationId, "send rejection")
                 .onSuccess(negotiation -> {
                     negotiation.transitionDeclined();
-                    update(negotiation, l -> l.preDeclined(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.declined(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionDeclining();
-                    update(negotiation, l -> l.preDeclining(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }
@@ -416,12 +416,12 @@ private BiConsumer<Object, Throwable> onAgreementSent(String id, ContractAgreeme
                 .onSuccess(negotiation -> {
                     negotiation.setContractAgreement(agreement);
                     negotiation.transitionConfirmed();
-                    update(negotiation, l -> l.preConfirmed(negotiation));
+                    negotiationStore.save(negotiation);
                     observable.invokeForEach(l -> l.confirmed(negotiation));
                 })
                 .onFailure(negotiation -> {
                     negotiation.transitionConfirming();
-                    update(negotiation, l -> l.preConfirming(negotiation));
+                    negotiationStore.save(negotiation);
                 })
                 .build();
     }

diff --git a/...g/eclipse/dataspaceconnector/contract/negotiation/ContractNegotiationIntegrationTest.java b/...g/eclipse/dataspaceconnector/contract/negotiation/ContractNegotiationIntegrationTest.java
@@ -34,7 +34,6 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -84,8 +83,6 @@ void testNegotiation_initialOfferAccepted() {
                     assertThat(consumerNegotiation.getLastContractOffer()).isEqualTo(providerNegotiation.getLastContractOffer());
                     assertThat(consumerNegotiation.getContractAgreement()).isEqualTo(providerNegotiation.getContractAgreement());
 
-                    // verify that the preConfirmed event has occurred twice - once for cons. once for prov
-                    verify(negotiationListenerMock, times(2)).preConfirmed(any());
 
                     verify(validationService, atLeastOnce()).validate(token, offer);
                     verify(validationService, atLeastOnce()).validate(eq(token), any(ContractAgreement.class), any(ContractOffer.class));
@@ -134,9 +131,6 @@ void testNegotiation_initialOfferDeclined() {
                     assertNegotiations(consumerNegotiation, providerNegotiation, 1);
                     assertThat(consumerNegotiation.getLastContractOffer()).isEqualTo(providerNegotiation.getLastContractOffer());
 
-                    // verify that the preConfirmed event has occurred twice - once for cons. once for prov
-                    verify(negotiationListenerMock, times(2)).preDeclined(any());
-
                     // Assert that no agreement has been stored on either side
                     assertThat(consumerNegotiation.getContractAgreement()).isNull();
                     assertThat(providerNegotiation.getContractAgreement()).isNull();
@@ -188,8 +182,6 @@ void testNegotiation_agreementDeclined() {
                     // Assert that no agreement has been stored on either side
                     assertThat(consumerNegotiation.getContractAgreement()).isNull();
                     assertThat(providerNegotiation.getContractAgreement()).isNull();
-                    // verify that the preConfirmed event has occurred twice - once for cons. once for prov
-                    verify(negotiationListenerMock, times(2)).preDeclined(any());
 
                     verify(validationService, atLeastOnce()).validate(token, offer);
                     verify(validationService, atLeastOnce()).validate(eq(token), any(ContractAgreement.class), any(ContractOffer.class));
@@ -252,7 +244,6 @@ void testNegotiation_counterOfferAccepted() {
                     verify(validationService, atLeastOnce()).validate(token, initialOffer);
                     verify(validationService, atLeastOnce()).validate(token, counterOffer, initialOffer);
                     verify(validationService, atLeastOnce()).validate(eq(token), any(ContractAgreement.class), any(ContractOffer.class));
-                    verify(negotiationListenerMock, times(2)).preConfirmed(any());
                 });
         // Stop provider and consumer negotiation managers
         providerManager.stop();
@@ -310,8 +301,6 @@ void testNegotiation_counterOfferDeclined() {
                     verify(validationService, atLeastOnce()).validate(token, initialOffer);
                     verify(validationService, atLeastOnce()).validate(token, counterOffer, initialOffer);
                     verify(validationService, atLeastOnce()).validate(eq(token), any(ContractAgreement.class), any(ContractOffer.class));
-
-                    verify(negotiationListenerMock, times(2)).preDeclined(any());
                 });
         // Stop provider and consumer negotiation managers
         providerManager.stop();
@@ -385,9 +374,6 @@ void testNegotiation_consumerCounterOfferAccepted() {
                     verify(validationService, atLeastOnce()).validate(token, counterOffer, initialOffer);
                     verify(validationService, atLeastOnce()).validate(token, consumerCounterOffer, counterOffer);
                     verify(validationService, atLeastOnce()).validate(eq(token), any(ContractAgreement.class), eq(consumerCounterOffer));
-
-                    verify(negotiationListenerMock, times(2)).preConfirmed(any());
-
                 });
         // Stop provider and consumer negotiation managers
         providerManager.stop();
@@ -458,7 +444,6 @@ void testNegotiation_consumerCounterOfferDeclined() {
                     verify(validationService, atLeastOnce()).validate(token, initialOffer);
                     verify(validationService, atLeastOnce()).validate(token, counterOffer, initialOffer);
                     verify(validationService, atLeastOnce()).validate(token, consumerCounterOffer, counterOffer);
-                    verify(negotiationListenerMock, times(2)).preDeclined(any());
                 });
         // Stop provider and consumer negotiation managers
         providerManager.stop();