Skip to content

updated test suite, old zip had some errors between test plan documen… #21

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

updated test suite, old zip had some errors between test plan documen… #21

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
155 changes: 68 additions & 87 deletions ...rvices/fhir-itb-service/src/main/java/eu/europa/ec/fhir/handlers/FhirProxyController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,144 +102,125 @@ private String getResourceTypeFromJson(String json, String resourceType) {

//private String get

@RequestMapping({"/proxy", "/proxy/*", "/proxy/*/{id}"})
@RequestMapping({
"/proxy", // system-level (e.g., Bundle POST)
"/proxy/{resourceType}", // e.g., /proxy/Patient?name=...
"/proxy/{resourceType}/", // trailing slash
"/proxy/{resourceType}/{id}", // e.g., /proxy/Patient/123
"/proxy/{resourceType}/_search", // POST-based search
"/proxy/{resourceType}/_search/" // trailing slash
})
public DeferredResult<ResponseEntity<String>> handleRequest(
HttpServletRequest request,
//@PathVariable(value = "resourceType", required = false) String resourceType,
@PathVariable(value = "id", required = false) Optional<String> resourceId,
@PathVariable(value = "resourceType", required = false) Optional<String> resourceTypeOpt,
@PathVariable(value = "id", required = false) Optional<String> resourceId,
@RequestBody(required = false) Optional<String> payload
) {
RequestParams proxyRequestParams;
List<Map.Entry<String, RequestParams>> testIds = new ArrayList<>();

// Retrieving the resourceType from the json payload
String resourceType = payload.map(body -> getResourceTypeFromJson(body, "resourceType")).orElse("");

RequestParams proxyRequestParams = null;

// Detect if this is a Bundle (system-level POST)
String resourceTypeFromBody = payload.map(b -> getResourceTypeFromJson(b, "resourceType")).orElse("");
boolean isBundle = "Bundle".equals(resourceTypeFromBody);

LOGGER.info("Checking resourceType for Bundle");
LOGGER.info("Resource type from json \"{}\"", resourceType);
List<Map.Entry<String, RequestParams>> testIds = new ArrayList<>();
// Resolve resourceType: prefer path variable, fall back to body only for Bundle
String resourceType = resourceTypeOpt.orElse(isBundle ? "Bundle" : "");

Comment on lines +127 to 128
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When resourceType is empty (not a Bundle), the downstream code will have issues. Line 173 logs a warning about this, but the code continues execution which may cause the 'actor' error mentioned in the comment. Consider returning an error response early when resourceType cannot be determined.

Suggested change
String resourceType = resourceTypeOpt.orElse(isBundle ? "Bundle" : "");
// Early return if resourceType cannot be determined (not a Bundle)
if (resourceType.isEmpty()) {
DeferredResult<ResponseEntity<String>> result = new DeferredResult<>();
result.setResult(ResponseEntity
.badRequest()
.body("{\"error\": \"resourceType could not be determined from path or body\"}"));
return result;
}

Copilot uses AI. Check for mistakes.
if ("Bundle".equals(resourceType)) {
// Special handling for POST /{type}/_search (body is usually form-encoded, not JSON)
boolean isPostSearch = request.getRequestURI().matches(".*/_search/?$");
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern .*/_search/?$ is a magic string that duplicates the logic from the @RequestMapping. Consider extracting this pattern as a constant or using a more maintainable approach to detect POST search requests.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern ".*/_search/?$" is a magic string that duplicates logic from the @RequestMapping. Consider extracting this pattern to a constant or using a more robust method to detect POST search requests.

Suggested change
boolean isPostSearch = request.getRequestURI().matches(".*/_search/?$");
boolean isPostSearch = request.getRequestURI().matches(POST_SEARCH_ENDPOINT_PATTERN);

Copilot uses AI. Check for mistakes.

// loop through all entries in the bundle
if (isBundle) {
// Your existing bundle logic (unchanged)
try {
JsonNode root = objectMapper.readTree(payload.get());
JsonNode root = objectMapper.readTree(payload.orElse("{}"));
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an empty JSON object "{}" as a fallback when payload is empty could cause issues since Bundle processing expects a proper Bundle structure. Consider using an empty string or handling the empty case explicitly.

Suggested change
JsonNode root = objectMapper.readTree(payload.orElse("{}"));
JsonNode root = objectMapper.readTree(payload.orElse(""));

Copilot uses AI. Check for mistakes.
JsonNode entries = root.at(this.fhirRefCodes.get("entry").get());

if (entries.isArray()) {
for (JsonNode entry : entries) {
String method = "";
String entryResourceType = "";
String vaccineCode = "";

// Get resource type for each entry
JsonNode resourceNode = entry.get("resource");
if (resourceNode != null && resourceNode.has("resourceType")) {
entryResourceType = resourceNode.get("resourceType").asText();
LOGGER.info("Found entry with resource type: {}", entryResourceType);

// Get vaccine code if resource type is Immunization
if (resourceNode.has("vaccineCode")) {
JsonNode vaccineCodeNode = resourceNode.get("vaccineCode");
if (vaccineCodeNode.has("coding")) {
JsonNode codingArray = vaccineCodeNode.get("coding");
if (codingArray.isArray() && codingArray.size() > 0) {
JsonNode firstCoding = codingArray.get(0);
if (firstCoding.has("code")) {
vaccineCode = firstCoding.get("code").asText();
LOGGER.info("Found vaccine code: {}", vaccineCode);
}
}
}
}
}
String entryResourceType = resourceNode != null && resourceNode.has("resourceType")
? resourceNode.get("resourceType").asText()
: "";

String method = Optional.ofNullable(entry.get("request"))
.map(r -> r.get("method"))
.map(JsonNode::asText)
.map(String::toLowerCase)
.orElse("");

// Get request method and URL for each entry
JsonNode requestNode = entry.get("request");
if (requestNode != null) {
if (requestNode.has("method")) {
method = requestNode.get("method").asText().toLowerCase();
LOGGER.info("Found request method: {}", method);
String vaccineCode = "";
if (resourceNode != null && resourceNode.has("vaccineCode")) {
JsonNode coding = resourceNode.path("vaccineCode").path("coding");
if (coding.isArray() && coding.size() > 0) {
vaccineCode = coding.get(0).path("code").asText("");
}
}

testIds.add(new AbstractMap.SimpleEntry<>(method + "-" + entryResourceType + "-" + vaccineCode, fhirProxyServiceHelper.toFhirHttpParams(request, "", Optional.of(resourceNode.toString()))));
LOGGER.info("Test ID: {}", method + "-" + entryResourceType + "-" + vaccineCode);
testIds.add(new AbstractMap.SimpleEntry<>(
method + "-" + entryResourceType + "-" + vaccineCode,
fhirProxyServiceHelper.toFhirHttpParams(request, "", Optional.ofNullable(resourceNode).map(JsonNode::toString))
));
}
}
} catch (JsonProcessingException e) {
LOGGER.warn("Failed to parse Bundle entries: {}", e.getMessage());
}

proxyRequestParams = fhirProxyServiceHelper.toFhirHttpParams(request, "", payload);


} else {
Comment on lines +133 to 169
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using payload.orElse("{}") when payload is empty will create an empty JSON object, but the subsequent code expects Bundle structure with 'entry' field. This could cause null pointer exceptions or unexpected behavior. The Bundle check should ensure payload is present before processing.

Copilot uses AI. Check for mistakes.
Optional<String> referenceCode = payload.flatMap(body -> getReferenceCode(body, resourceType));
if (resourceType.isBlank()) {
// We could not determine the resource type; this causes the “actor” error downstream.
// Bail out early with a clear log (or consider mapping to a default/“general” actor).
LOGGER.warn("Resource type is missing. Ensure @RequestMapping uses {resourceType} instead of *.");
Comment on lines +172 to +173
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code logs a warning when resourceType is blank but continues execution. This could lead to downstream errors. Consider returning an appropriate error response or throwing an exception to fail fast.

Suggested change
// Bail out early with a clear log (or consider mapping to a default/“general” actor).
LOGGER.warn("Resource type is missing. Ensure @RequestMapping uses {resourceType} instead of *.");
// Bail out early with a clear log and error response.
LOGGER.warn("Resource type is missing. Ensure @RequestMapping uses {resourceType} instead of *.");
return ResponseEntity.badRequest().body("Resource type is missing or invalid. Ensure @RequestMapping uses {resourceType} instead of *.");

Copilot uses AI. Check for mistakes.
}

// Build the path passed to the proxy target (/Patient, /Patient/123, /Patient/_search, etc.)
String fullPath;
if (isPostSearch) {
fullPath = resourceType + "/_search";
} else if (resourceId.isPresent()) {
fullPath = resourceType + "/" + resourceId.get();
} else {
fullPath = resourceType; // e.g., GET /Patient?name=...
}

String fullPath = String.format("%s%s", resourceType, resourceId.map(value -> "/" + value).orElse(""));
proxyRequestParams = fhirProxyServiceHelper.toFhirHttpParams(request, fullPath, payload);

testIds.add(new AbstractMap.SimpleEntry<>(String.format("%s-%s%s",
Optional<String> referenceCode = payload.flatMap(body -> getReferenceCode(body, resourceType));
String testKey = String.format("%s-%s%s",
proxyRequestParams.method().toString().toLowerCase(),
resourceType.replace("/", ""),
referenceCode.map(code -> "-" + code).orElse("")), proxyRequestParams));
referenceCode.map(code -> "-" + code).orElse("")
);
testIds.add(new AbstractMap.SimpleEntry<>(testKey, proxyRequestParams));
}




LOGGER.info("Starting test session(s) for \"{}\"", testIds);
// forwards the request to the FHIR ACC server.
var deferredResult = new DeferredResult<ResponseEntity<String>>();
var deferredRequest = new DeferredRequest(proxyRequestParams, deferredResult);

for (Entry<String, RequestParams> testId : testIds) {
for (Map.Entry<String, RequestParams> testId : testIds) {
try {
// start test sessions and defer the request
var startSessionPayload = StartSessionRequestPayload.fromRequestParams(new String[]{testId.getKey()}, testId.getValue());
var startSessionPayload = StartSessionRequestPayload.fromRequestParams(
new String[]{testId.getKey()}, testId.getValue());
var itbResponse = itbRestClient.startSession(startSessionPayload);
var createdSessions = itbResponse.createdSessions();
var sessionId = createdSessions[0].session();

LOGGER.info("Test session(s) created: {}", (Object[]) createdSessions);
var sessionId = itbResponse.createdSessions()[0].session();
deferredRequests.put(sessionId, deferredRequest);

} catch (Exception e) {
LOGGER.warn("Failed to start test session(s) for testId {}: {}", testId, e.getMessage());
deferredRequest.resolve();


// In case of failure trigger the general suite to check for issues.
// No code included, method - resourceType only
try {
String generalTestId = String.format("%s-%s",
proxyRequestParams.method().toString().toLowerCase(),
resourceType.replace("/", "")
);

LOGGER.info("Initiating general test session(s), testId:" + testId.getKey().replaceAll("-[^-]*$", ""));

var startSessionPayload = StartSessionRequestPayload.fromRequestParams(new String[]{testId.getKey().replaceAll("-[^-]*$", "")}, testId.getValue());
var startSessionPayload = StartSessionRequestPayload.fromRequestParams(
new String[]{testId.getKey().replaceAll("-[^-]*$", "")}, testId.getValue());
var itbResponse = itbRestClient.startSession(startSessionPayload);
var createdSessions = itbResponse.createdSessions();
var sessionId = createdSessions[0].session();

LOGGER.info("Test session(s) created: {}", (Object[]) createdSessions);
var sessionId = itbResponse.createdSessions()[0].session();
deferredRequests.put(sessionId, deferredRequest);

} catch (Exception ec) {
LOGGER.warn("Failed to start general test session(s): {}", ec.getMessage());
deferredRequest.resolve();
}
}


}

return deferredResult;


}
}
}