-
Notifications
You must be signed in to change notification settings - Fork 0
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
PRMT-3413 E2E testing - Unexpected Interaction ID and NHS number #21
Open
joefong-nhs
wants to merge
21
commits into
main
Choose a base branch
from
PRMT-3413
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
0ff49b1
[PRMT-3413] Add method to queue helper to check that a message does n…
joefong-nhs b746f75
[PRMT-3413] Add url of ehr out service to test config
joefong-nhs c72be7f
[PRMT-3413] Add asid code to the Gp2gp system class
joefong-nhs 0264513
[PRMT-3413] Add an EhrRequestMessage class and its builder for making…
joefong-nhs 83d6646
[PRMT-3413] Fixed the incoherent conversation ids in RCMR_IN010000UK0…
joefong-nhs 73221e1
[PRMT-3413] Added two E2E tests, one for unrecognised interaction ID …
joefong-nhs e548cd8
[PRMT-3413] Allow health check to wait for 2xx response for at most 5…
joefong-nhs 7266c1c
[PRMT-3413] Change the invalid interactionId used in test to make it …
joefong-nhs a2972cd
[PRMT-3413] Add test for Erroneous message: COPC continue request wit…
joefong-nhs ebaa402
[PRMT-3413] Bundled test cases into one ParameterizedTest
joefong-nhs c349de9
[PRMT-3413] Change healthcheck class to use LOGGER.info rather than s…
joefong-nhs e8c1fae
[PRMT-3413] Remove duplicated tests, minor fix on log wordings and te…
joefong-nhs a2f073b
[PRMT-3413] Minor fix on wordings
joefong-nhs 6e37ee4
[PRMT-3413] Move the list of known asid codes to a static final field
joefong-nhs 956dfa7
[PRMT-3413] Amended a misleading log message
joefong-nhs 3f7a68d
[PRMT-3413] Amend incorrect LOGGER setting in new class
joefong-nhs f3cc475
[PRMT-3413] Address comments (change some final class to just public …
joefong-nhs 8f6eaf9
[PRMT-3413] Address comments and add a static method getUuidAsUpperCa…
joefong-nhs a34a5c9
[PRMT-3413] Addressed PR comment (getUUIDAsUpperCaseString)
joefong-nhs 161bc2d
[PRMT-3413] Address PR comment
joefong-nhs ea8bd7c
[PRMT-3413] Address PR comment https://github.com/nhsconnect/prm-repo…
joefong-nhs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
src/test/java/uk/nhs/prm/deduction/e2e/models/ContinueRequestMessage.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package uk.nhs.prm.deduction.e2e.models; | ||
|
||
import uk.nhs.prm.deduction.e2e.utility.Resources; | ||
|
||
import java.util.UUID; | ||
|
||
import static uk.nhs.prm.deduction.e2e.utility.TestUtils.getUUIDAsUpperCaseString; | ||
|
||
public class ContinueRequestMessage { | ||
private UUID conversationId; | ||
private UUID messageId; | ||
private String sourceGpOds; | ||
private String destinationGpOds; | ||
private String sourceGpAsid; | ||
private String destinationGpAsid; | ||
|
||
public ContinueRequestMessage(UUID conversationId, UUID messageId, String sourceGpOds, String destinationGpOds, String sourceGpAsid, String destinationGpAsid) { | ||
this.conversationId = conversationId; | ||
this.messageId = messageId; | ||
this.sourceGpOds = sourceGpOds; | ||
this.destinationGpOds = destinationGpOds; | ||
this.sourceGpAsid = sourceGpAsid; | ||
this.destinationGpAsid = destinationGpAsid; | ||
} | ||
|
||
public String toJsonString() { | ||
return Resources.readTestResourceFile("COPC_IN000001UK01") | ||
.replaceAll("DBC31D30-F984-11ED-A4C4-956AA80C6B4E", conversationId()) | ||
.replaceAll("DE304CA0-F984-11ED-808B-AC162D1F16F0", messageId()) | ||
.replaceAll("B85002", sourceGpOds) | ||
.replaceAll("200000001613", sourceGpAsid) | ||
.replaceAll("M85019", destinationGpOds) | ||
.replaceAll("200000000149", destinationGpAsid); | ||
} | ||
|
||
public String conversationId() { | ||
return getUUIDAsUpperCaseString(conversationId); | ||
} | ||
|
||
public String messageId() { | ||
return getUUIDAsUpperCaseString(messageId); | ||
} | ||
} |
91 changes: 91 additions & 0 deletions
91
src/test/java/uk/nhs/prm/deduction/e2e/models/ContinueRequestMessageBuilder.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
package uk.nhs.prm.deduction.e2e.models; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import uk.nhs.prm.deduction.e2e.TestConfiguration; | ||
import uk.nhs.prm.deduction.e2e.utility.TestUtils; | ||
|
||
import java.util.UUID; | ||
|
||
public class ContinueRequestMessageBuilder { | ||
private UUID conversationId; | ||
private UUID messageId; | ||
private String sourceGpOds; | ||
private String destinationGpOds; | ||
private String sourceGpAsid; | ||
private String destinationGpAsid; | ||
|
||
private static final Logger LOGGER = LogManager.getLogger(ContinueRequestMessageBuilder.class); | ||
|
||
AndyFlintNHS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public ContinueRequestMessageBuilder() { | ||
withRandomlyGeneratedConversationId(); | ||
withRandomlyGeneratedMessageId(); | ||
withEhrSourceGp(Gp2GpSystem.REPO_DEV); | ||
withEhrDestinationGp(Gp2GpSystem.TPP_PTL_INT); | ||
} | ||
|
||
public ContinueRequestMessageBuilder withConversationId(UUID conversationId) { | ||
this.conversationId = conversationId; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withRandomlyGeneratedConversationId() { | ||
this.conversationId = UUID.randomUUID(); | ||
LOGGER.info("generated conversation id {}", conversationId); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withEhrSourceAsRepo(TestConfiguration config) { | ||
Gp2GpSystem repoInEnv = Gp2GpSystem.repoInEnv(config); | ||
this.sourceGpOds = repoInEnv.odsCode(); | ||
this.sourceGpAsid = repoInEnv.asidCode(); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withEhrSourceGp(Gp2GpSystem ehrSourceGp) { | ||
this.sourceGpOds = ehrSourceGp.odsCode(); | ||
this.sourceGpAsid = ehrSourceGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withEhrDestinationGp(Gp2GpSystem ehrDestinationGp) { | ||
this.destinationGpOds = ehrDestinationGp.odsCode(); | ||
this.destinationGpAsid = ehrDestinationGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withMessageId(UUID messageId) { | ||
this.messageId = messageId; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withRandomlyGeneratedMessageId() { | ||
this.messageId = UUID.randomUUID(); | ||
LOGGER.info("generated message id {}", messageId); | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withSourceGpOds(String sourceGpOds) { | ||
this.sourceGpOds = sourceGpOds; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withDestinationGpOds(String destinationGpOds) { | ||
this.destinationGpOds = destinationGpOds; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withSourceGpAsid(String sourceGpAsid) { | ||
this.sourceGpAsid = sourceGpAsid; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessageBuilder withDestinationGpAsid(String destinationGpAsid) { | ||
this.destinationGpAsid = destinationGpAsid; | ||
return this; | ||
} | ||
|
||
public ContinueRequestMessage build() { | ||
return new ContinueRequestMessage(conversationId, messageId, sourceGpOds, destinationGpOds, sourceGpAsid, destinationGpAsid); | ||
} | ||
} |
46 changes: 46 additions & 0 deletions
46
src/test/java/uk/nhs/prm/deduction/e2e/models/EhrRequestMessage.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
package uk.nhs.prm.deduction.e2e.models; | ||
|
||
import uk.nhs.prm.deduction.e2e.utility.Resources; | ||
|
||
import java.util.UUID; | ||
|
||
import static uk.nhs.prm.deduction.e2e.utility.TestUtils.getUUIDAsUpperCaseString; | ||
|
||
public class EhrRequestMessage { | ||
private final String nhsNumber; | ||
private final String sourceGpOds; | ||
private final String destinationGpOds; | ||
private final String sourceGpAsid; | ||
private final String destinationGpAsid; | ||
private final UUID conversationId; | ||
private final UUID messageId; | ||
|
||
public EhrRequestMessage(String nhsNumber, String sourceGpOds, String destinationGpOds, String sourceGpAsid, String destinationGpAsid, UUID conversationId, UUID messageId) { | ||
this.nhsNumber = nhsNumber; | ||
this.sourceGpOds = sourceGpOds; | ||
this.destinationGpOds = destinationGpOds; | ||
this.sourceGpAsid = sourceGpAsid; | ||
this.destinationGpAsid = destinationGpAsid; | ||
this.conversationId = conversationId; | ||
this.messageId = messageId; | ||
} | ||
|
||
public String toJsonString() { | ||
return Resources.readTestResourceFile("RCMR_IN010000UK05") | ||
.replaceAll("9692842304", nhsNumber) | ||
.replaceAll("17a757f2-f4d2-444e-a246-9cb77bef7f22", conversationId()) | ||
.replaceAll("C445C720-B0EB-4E36-AF8A-48CD1CA5DE4F", messageId()) | ||
.replaceAll("B86041", sourceGpOds) | ||
.replaceAll("200000001161", sourceGpAsid) | ||
.replaceAll("A91720", destinationGpOds) | ||
.replaceAll("200000000631", destinationGpAsid); | ||
} | ||
|
||
public String conversationId() { | ||
return getUUIDAsUpperCaseString(conversationId); | ||
} | ||
|
||
public String messageId() { | ||
return getUUIDAsUpperCaseString(messageId); | ||
} | ||
} |
83 changes: 83 additions & 0 deletions
83
src/test/java/uk/nhs/prm/deduction/e2e/models/EhrRequestMessageBuilder.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
package uk.nhs.prm.deduction.e2e.models; | ||
|
||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
import uk.nhs.prm.deduction.e2e.TestConfiguration; | ||
import uk.nhs.prm.deduction.e2e.tests.Patient; | ||
import uk.nhs.prm.deduction.e2e.utility.TestUtils; | ||
|
||
import java.util.UUID; | ||
|
||
public class EhrRequestMessageBuilder { | ||
private String nhsNumber; | ||
private String sourceGpOds; | ||
private String destinationGpOds; | ||
private String sourceGpAsid; | ||
private String destinationGpAsid; | ||
private UUID conversationId; | ||
private UUID messageId; | ||
|
||
private static final Logger LOGGER = LogManager.getLogger(EhrRequestMessageBuilder.class); | ||
|
||
public EhrRequestMessageBuilder() { | ||
withRandomlyGeneratedConversationId(); | ||
withRandomlyGeneratedMessageId(); | ||
withNhsNumber("9692842304"); // nhs number in test template file | ||
withEhrSourceGp(Gp2GpSystem.REPO_DEV); | ||
withEhrDestinationGp(Gp2GpSystem.TPP_PTL_INT); | ||
} | ||
|
||
public EhrRequestMessageBuilder withNhsNumber(String nhsNumber) { | ||
this.nhsNumber = nhsNumber; | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withPatient(Patient patient) { | ||
return withNhsNumber(patient.nhsNumber()); | ||
} | ||
|
||
public EhrRequestMessageBuilder withEhrSourceAsRepo(TestConfiguration config) { | ||
Gp2GpSystem repoInEnv = Gp2GpSystem.repoInEnv(config); | ||
this.sourceGpOds = repoInEnv.odsCode(); | ||
this.sourceGpAsid = repoInEnv.asidCode(); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withEhrSourceGp(Gp2GpSystem ehrSourceGp) { | ||
this.sourceGpOds = ehrSourceGp.odsCode(); | ||
this.sourceGpAsid = ehrSourceGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withEhrDestinationGp(Gp2GpSystem ehrDestinationGp) { | ||
this.destinationGpOds = ehrDestinationGp.odsCode(); | ||
this.destinationGpAsid = ehrDestinationGp.asidCode(); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withConversationId(UUID conversationId) { | ||
this.conversationId = conversationId; | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withRandomlyGeneratedConversationId() { | ||
conversationId = UUID.randomUUID(); | ||
LOGGER.info("generated conversation id {}", conversationId); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withMessageId(UUID messageId) { | ||
this.messageId = messageId; | ||
return this; | ||
} | ||
|
||
public EhrRequestMessageBuilder withRandomlyGeneratedMessageId() { | ||
messageId = UUID.randomUUID(); | ||
LOGGER.info("generated message id {}", messageId); | ||
return this; | ||
} | ||
|
||
public EhrRequestMessage build() { | ||
return new EhrRequestMessage(nhsNumber, sourceGpOds, destinationGpOds, sourceGpAsid, destinationGpAsid, conversationId, messageId); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it's worth us breaking the
conversationId
andmessageId
methods out into a static method on a utility class. This UUID as upper case string format is useful across more than just thecontinueRequestMessage
class and rather than us duplicating a lot of code, having something like this might help:Which could be called in the code like this (using line 34 as an example):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I have added the static method getUuidAsUpperCaseString to TestUtils class and call it for conversation ids and message ids.
Later could you have a look and see if I did it right?
Because conversation id is a private field so I keep the getter methods, but now all those getter would call to our getUuidAsUpperCaseString :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest you don't even need the 'conversationId()' method at all. conversationId IS a private field as you say, but you don't need to use getters to reference private variables in the scope of the class where they're defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I overlooked this comment...
For
conversationId()
, in the e2e test that is a part that we need the conversation id from the message (the test use the conversation id to find an expected message in outbound queue), so I have kept the getter method for this purpose.