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.

Sign up for GitHub

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

cleanup: remove unused pre-methods from the ContractNegotiationListener #1659

Merged
merged 3 commits into from
Jul 22, 2022
Merged

cleanup: remove unused pre-methods from the ContractNegotiationListener #1659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cfdbe31
cleanup: remove unused pre methods (Events) from the ContractNegotiat…
janpmeyer Jul 13, 2022
256a43b
fix: fix codestyle and add inlining for 'update' method
janpmeyer Jul 13, 2022
ae3f17d
refactor: inline update method and clean unused import
janpmeyer Jul 15, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ in the detailed section referring to by linking pull requests or issues.
* Remove modules `:extensions:transfer-functions:transfer-functions-spi` 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

Expand Down
8 changes: 1 addition & 7 deletions ...g/eclipse/dataspaceconnector/contract/negotiation/AbstractContractNegotiationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 14 additions & 14 deletions ...lipse/dataspaceconnector/contract/negotiation/ConsumerContractNegotiationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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.",
Expand Down Expand Up @@ -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);
Expand All @@ -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())));
Expand All @@ -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())));
Expand Down Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down
22 changes: 11 additions & 11 deletions ...lipse/dataspaceconnector/contract/negotiation/ProviderContractNegotiationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
Expand Down Expand Up @@ -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.",
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())));
Expand All @@ -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())));

Expand Down Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand Down
15 changes: 0 additions & 15 deletions ...g/eclipse/dataspaceconnector/contract/negotiation/ContractNegotiationIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Loading