From dd528fe3b8ef9666e13f997d1fccb1e2050d7e42 Mon Sep 17 00:00:00 2001 From: Angshuman Sarkar Date: Sun, 26 Apr 2020 21:39:53 +0530 Subject: [PATCH 1/2] refactored for removing duplicates. doing rollback on error. renamed repository methodd process to grantConsentRequest --- .../ConsentArtefactQueryGenerator.java | 11 +++-- .../consent/ConsentArtefactRepository.java | 32 +++++++------ .../consent/ConsentManager.java | 14 +++--- .../consent/TransactionContext.java | 46 ++++++++++++------- .../ConsentRequestUserJourneyTest.java | 6 +-- 5 files changed, 63 insertions(+), 46 deletions(-) diff --git a/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactQueryGenerator.java b/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactQueryGenerator.java index 3503a546e..05d6e729a 100644 --- a/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactQueryGenerator.java +++ b/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactQueryGenerator.java @@ -43,12 +43,13 @@ public Mono toQueries(String requestId, JsonObject.mapFrom(hipConsentArtefact.getConsentDetail()), consentArtefactSignature, ConsentStatus.GRANTED.toString())); - Query updateConsentReqStatus = new Query(UPDATE_CONSENT_REQUEST_STATUS_QUERY, - Tuple.of(ConsentStatus.GRANTED.toString(), - LocalDateTime.now(), - requestId)); + //TODO: are we creating duplicate update query for consent-request? +// Query updateConsentReqStatus = new Query(UPDATE_CONSENT_REQUEST_STATUS_QUERY, +// Tuple.of(ConsentStatus.GRANTED.toString(), +// LocalDateTime.now(), +// requestId)); return Mono.just(QueryRepresentation.builder() - .queries(List.of(insertCA, insertHIPCA, updateConsentReqStatus)) + .queries(List.of(insertCA, insertHIPCA)) .hipConsentArtefactRepresentations(List.of(hipConsentArtefact)) .build()); } diff --git a/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactRepository.java b/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactRepository.java index 453724e16..984f5b3c2 100644 --- a/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactRepository.java +++ b/src/main/java/in/projecteka/consentmanager/consent/ConsentArtefactRepository.java @@ -18,6 +18,7 @@ import java.time.LocalDateTime; import java.time.ZoneId; +import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.stream.StreamSupport; @@ -38,20 +39,28 @@ public class ConsentArtefactRepository { "date_modified=$2 WHERE consent_artefact_id=$3"; private static final String FAILED_TO_RETRIEVE_CA = "Failed to retrieve Consent Artifact."; private static final String FAILED_TO_SAVE_CONSENT_ARTEFACT = "Failed to save consent artefact"; + private static final String FAILED_TO_UPDATE_STATUS = "Failed to update status"; + private static final String CAN_NOT_INITIALIZE_TRANSACTION = "Can not get connection."; private final PgPool dbClient; - public Mono process(List queries) { - return doInTransaction(queries); + public Mono grantConsentRequest(String requestId, List artefactQueries) { + List queriesInTransaction = new ArrayList<>(artefactQueries); + queriesInTransaction.add(new Query(UPDATE_CONSENT_REQUEST_STATUS_QUERY, + Tuple.of(ConsentStatus.GRANTED.toString(), + LocalDateTime.now(), + requestId))); + return doInTransaction(queriesInTransaction, FAILED_TO_SAVE_CONSENT_ARTEFACT); + //return doInTransaction(artefactQueries, FAILED_TO_SAVE_CONSENT_ARTEFACT); } - private Mono doInTransaction(List queries) { + private Mono doInTransaction(List queries, String contextErrorMessage) { return Mono.create(monoSink -> dbClient.begin(connectionAttempt -> { if (connectionAttempt.succeeded()) { - TransactionContext context = new TransactionContext(connectionAttempt.result(), monoSink); - context.executeInTransaction(queries.iterator(), FAILED_TO_SAVE_CONSENT_ARTEFACT); + TransactionContext context = new TransactionContext(connectionAttempt.result(), monoSink, contextErrorMessage); + context.executeInTransaction(queries.iterator()); } else { - monoSink.error(new RuntimeException("Can not get connectionAttempt to storage.")); + monoSink.error(new RuntimeException(CAN_NOT_INITIALIZE_TRANSACTION)); } })); } @@ -124,15 +133,8 @@ public Flux getConsentArtefacts(String consentRequestId) { } public Mono updateStatus(String consentId, String consentRequestId, ConsentStatus status) { - return Mono.create(monoSink -> dbClient.begin(connectionAttempt -> { - List queries = getUpdateQueries(consentId, consentRequestId, status); - if (connectionAttempt.succeeded()) { - TransactionContext context = new TransactionContext(connectionAttempt.result(), monoSink); - context.executeInTransaction(queries.iterator(), "Failed to update status"); - } else { - monoSink.error(new RuntimeException("Can not get connectionAttempt to storage.")); - } - })); + List queries = getUpdateQueries(consentId, consentRequestId, status); + return doInTransaction(queries, FAILED_TO_UPDATE_STATUS); } private List getUpdateQueries(String consentId, String consentRequestId, ConsentStatus status) { diff --git a/src/main/java/in/projecteka/consentmanager/consent/ConsentManager.java b/src/main/java/in/projecteka/consentmanager/consent/ConsentManager.java index ff7a77539..21c599727 100644 --- a/src/main/java/in/projecteka/consentmanager/consent/ConsentManager.java +++ b/src/main/java/in/projecteka/consentmanager/consent/ConsentManager.java @@ -205,16 +205,16 @@ private Mono> generateConsentArtefacts(St List grantedConsents, String patientId, ConsentRequestDetail consentRequest) { - return getAllQueries(requestId, grantedConsents, patientId, consentRequest) - .map(caQueries -> caQueries.stream().reduce(QueryRepresentation::add).get()) - .flatMap(queryRepresentation -> consentArtefactRepository.process(queryRepresentation.getQueries()) + return artefactRepresentations(requestId, grantedConsents, patientId, consentRequest) + .map(representations -> representations.stream().reduce(QueryRepresentation::add).get()) + .flatMap(queryRepresentation -> consentArtefactRepository.grantConsentRequest(requestId, queryRepresentation.getQueries()) .thenReturn(queryRepresentation.getHipConsentArtefactRepresentations())); } - private Mono> getAllQueries(String requestId, - List grantedConsents, - String patientId, - ConsentRequestDetail consentRequest) { + private Mono> artefactRepresentations(String requestId, + List grantedConsents, + String patientId, + ConsentRequestDetail consentRequest) { return Flux.fromIterable(grantedConsents) .flatMap(grantedConsent -> toConsentArtefact(consentRequest, grantedConsent) .flatMap(consentArtefact -> consentArtefactQueryGenerator.toQueries(requestId, diff --git a/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java b/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java index 3a4dd7f24..e4c02acb6 100644 --- a/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java +++ b/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java @@ -1,6 +1,9 @@ package in.projecteka.consentmanager.consent; import in.projecteka.consentmanager.consent.model.Query; +import io.vertx.core.AsyncResult; +import io.vertx.sqlclient.Row; +import io.vertx.sqlclient.RowSet; import io.vertx.sqlclient.Transaction; import reactor.core.publisher.MonoSink; @@ -9,10 +12,12 @@ public class TransactionContext { private Transaction transaction; private MonoSink sink; + private final String txErrorMsg; - public TransactionContext(Transaction transaction, MonoSink sink) { + public TransactionContext(Transaction transaction, MonoSink sink, String txErrorMsg) { this.transaction = transaction; this.sink = sink; + this.txErrorMsg = txErrorMsg; } private void commit() { @@ -20,31 +25,40 @@ private void commit() { if (result.succeeded()) { this.sink.success(); } else { - error(new RuntimeException(result.cause().getMessage())); + error(new RuntimeException(this.txErrorMsg, result.cause())); } }); } private void error(RuntimeException e) { + this.transaction.rollback(); this.sink.error(e); } - public void executeInTransaction(Iterator iterator, String message) { + public void executeInTransaction(Iterator iterator) { if (iterator.hasNext()) { Query query = iterator.next(); - transaction.preparedQuery(query.getQueryString()) - .execute(query.getParams(), - handler -> { - if (handler.succeeded()) { - if (iterator.hasNext()) { - executeInTransaction(iterator, message); - } else { - commit(); - } - } else { - error(new RuntimeException(message)); - } - }); + boolean hasParams = query.getParams() != null && query.getParams().size() > 0; + if (!hasParams) { + transaction.preparedQuery(query.getQueryString()) + .execute(handler -> handleResponseAndExecNext(handler, iterator)); + } else { + transaction.preparedQuery(query.getQueryString()) + .execute(query.getParams(), + handler -> handleResponseAndExecNext(handler, iterator)); + } + } + } + + private void handleResponseAndExecNext(AsyncResult> handler, Iterator iterator) { + if (handler.succeeded()) { + if (iterator.hasNext()) { + executeInTransaction(iterator); + } else { + commit(); + } + } else { + error(new RuntimeException(this.txErrorMsg, handler.cause())); } } } diff --git a/src/test/java/in/projecteka/consentmanager/consent/ConsentRequestUserJourneyTest.java b/src/test/java/in/projecteka/consentmanager/consent/ConsentRequestUserJourneyTest.java index f17ac11da..27bf2eb24 100644 --- a/src/test/java/in/projecteka/consentmanager/consent/ConsentRequestUserJourneyTest.java +++ b/src/test/java/in/projecteka/consentmanager/consent/ConsentRequestUserJourneyTest.java @@ -29,7 +29,6 @@ import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.http.MediaType; -import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.test.web.reactive.server.WebTestClient; @@ -49,6 +48,7 @@ import static in.projecteka.consentmanager.consent.model.ConsentStatus.REQUESTED; import static java.lang.String.format; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -332,7 +332,7 @@ public void shouldApproveConsentGrant() throws JsonProcessingException { .thenReturn(Mono.just(consentRequestDetail)); when(pinVerificationTokenService.validateToken(token)) .thenReturn(Mono.just(new Caller(patientId, false))); - when(consentArtefactRepository.process(any())).thenReturn(Mono.empty()); + when(consentArtefactRepository.grantConsentRequest(eq("30d02f6d-de17-405e-b4ab-d31b2bb799d7"), any())).thenReturn(Mono.empty()); when(consentNotificationPublisher.publish(any())).thenReturn(Mono.empty()); webTestClient.post() @@ -394,7 +394,7 @@ public void shouldNotApproveConsentGrantForInvalidCareContext() throws JsonProce .thenReturn(Mono.just(new Caller(patientId, false))); when(repository.requestOf("30d02f6d-de17-405e-b4ab-d31b2bb799d7", "REQUESTED", patientId)) .thenReturn(Mono.just(consentRequestDetail)); - when(consentArtefactRepository.process(any())).thenReturn(Mono.empty()); + when(consentArtefactRepository.grantConsentRequest(eq("30d02f6d-de17-405e-b4ab-d31b2bb799d"), any())).thenReturn(Mono.empty()); when(consentNotificationPublisher.publish(any())).thenReturn(Mono.empty()); webTestClient.post() From eecd488d60307bb9d6eae8906e1a30c58a87bd13 Mon Sep 17 00:00:00 2001 From: Angshuman Sarkar Date: Sun, 26 Apr 2020 23:45:53 +0530 Subject: [PATCH 2/2] removing explicit rollback. Seems vertx already rollsback in case of error! --- .../consentmanager/consent/TransactionContext.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java b/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java index e4c02acb6..b0193a6bb 100644 --- a/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java +++ b/src/main/java/in/projecteka/consentmanager/consent/TransactionContext.java @@ -31,22 +31,16 @@ private void commit() { } private void error(RuntimeException e) { - this.transaction.rollback(); + //this.transaction.rollback(); this.sink.error(e); } public void executeInTransaction(Iterator iterator) { if (iterator.hasNext()) { Query query = iterator.next(); - boolean hasParams = query.getParams() != null && query.getParams().size() > 0; - if (!hasParams) { - transaction.preparedQuery(query.getQueryString()) - .execute(handler -> handleResponseAndExecNext(handler, iterator)); - } else { - transaction.preparedQuery(query.getQueryString()) - .execute(query.getParams(), - handler -> handleResponseAndExecNext(handler, iterator)); - } + transaction.preparedQuery(query.getQueryString()) + .execute(query.getParams(), + handler -> handleResponseAndExecNext(handler, iterator)); } }