diff --git a/apps/bfd-server/bfd-server-shared-utils/src/main/java/gov/cms/bfd/server/sharedutils/BfdMDC.java b/apps/bfd-server/bfd-server-shared-utils/src/main/java/gov/cms/bfd/server/sharedutils/BfdMDC.java index bb07c734d4..86899b4b13 100644 --- a/apps/bfd-server/bfd-server-shared-utils/src/main/java/gov/cms/bfd/server/sharedutils/BfdMDC.java +++ b/apps/bfd-server/bfd-server-shared-utils/src/main/java/gov/cms/bfd/server/sharedutils/BfdMDC.java @@ -262,6 +262,12 @@ public class BfdMDC { /** MDC key for the resources returned. */ public static final String RESOURCES_RETURNED = computeMDCKey("resources_returned_count"); + /** MDC key for MBI Hash. */ + public static final String MBI_HASH = "mbi_hash"; + + /** MDC key for MBI Id. */ + public static final String MBI_ID = "mbi_id"; + /** * Set the MDC Adapter. Normally this is not needed, but it can be useful in testing. * diff --git a/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/TransformerUtilsV2.java b/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/TransformerUtilsV2.java index accae7893e..d5a630be23 100644 --- a/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/TransformerUtilsV2.java +++ b/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/TransformerUtilsV2.java @@ -34,7 +34,6 @@ import gov.cms.bfd.model.rif.entities.SNFClaimColumn; import gov.cms.bfd.model.rif.entities.SNFClaimLine; import gov.cms.bfd.model.rif.parse.InvalidRifValueException; -import gov.cms.bfd.server.sharedutils.BfdMDC; import gov.cms.bfd.server.war.commons.C4BBInstutionalClaimSubtypes; import gov.cms.bfd.server.war.commons.CCWProcedure; import gov.cms.bfd.server.war.commons.CCWUtils; @@ -4058,17 +4057,6 @@ static void setProviderNumber(ExplanationOfBenefit eob, String providerNumber) { CcwCodebookVariable.PRVDR_NUM, providerNumber))); } - /** - * Logs the mbi hash to mdc. - * - * @param mbiHash the mbi hash to log - */ - public static void logMbiHashToMdc(String mbiHash) { - if (!Strings.isNullOrEmpty(mbiHash)) { - BfdMDC.put("mbi_hash", mbiHash); - } - } - /** * Compares {@link LocalDate} a against {@link LocalDate} using the supplied {@link * ParamPrefixEnum}. diff --git a/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/pac/AbstractR4ResourceProvider.java b/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/pac/AbstractR4ResourceProvider.java index 45155b1bcc..950c80b605 100644 --- a/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/pac/AbstractR4ResourceProvider.java +++ b/apps/bfd-server/bfd-server-war/src/main/java/gov/cms/bfd/server/war/r4/providers/pac/AbstractR4ResourceProvider.java @@ -21,6 +21,10 @@ import com.codahale.metrics.MetricRegistry; import com.google.common.annotations.VisibleForTesting; import com.newrelic.api.agent.Trace; +import gov.cms.bfd.model.rda.Mbi; +import gov.cms.bfd.model.rda.entities.RdaFissClaim; +import gov.cms.bfd.model.rda.entities.RdaMcsClaim; +import gov.cms.bfd.server.sharedutils.BfdMDC; import gov.cms.bfd.server.war.commons.AbstractResourceProvider; import gov.cms.bfd.server.war.commons.OffsetLinkBuilder; import gov.cms.bfd.server.war.commons.OpenAPIContentProvider; @@ -30,13 +34,13 @@ import gov.cms.bfd.server.war.r4.providers.pac.common.ResourceTransformer; import gov.cms.bfd.server.war.r4.providers.pac.common.ResourceTypeV2; import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; import jakarta.annotation.PostConstruct; import jakarta.persistence.EntityManager; import jakarta.persistence.NoResultException; import jakarta.persistence.PersistenceContext; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -232,6 +236,9 @@ public T read(@IdParam IdType claimId, RequestDetails requestDetails) { throw new ResourceNotFoundException(claimId); } + Mbi claimEntityMbi = getClaimEntityMbi(claimIdObj.getRight(), claimEntity); + if (claimEntityMbi != null) logMbiIdentifiersToMdc(claimEntityMbi); + return transformEntity(claimIdObj.getRight(), claimEntity, includeTaxNumbers); } @@ -282,6 +289,41 @@ private T transformEntity( } } + /** + * Returns the {@link Mbi} associated with the given {@link RdaFissClaim} or {@link RdaMcsClaim}, + * depending on resource type. + * + * @param generic type extending {@link IBaseResource} + * @param claimIdType the {@link ResourceTypeV2} indicating what the claim's type is (either fiss + * or mcs) + * @param claimEntity the claim entity itself; either a {@link RdaFissClaim} or {@link + * RdaMcsClaim} + * @return the {@link Mbi} associated with the given claim entity + * @throws IllegalArgumentException if the given {@link ResourceTypeV2} does not indicate either a + * FISS or MCS claim ID type + */ + private @Nullable Mbi getClaimEntityMbi( + ResourceTypeV2 claimIdType, Object claimEntity) { + return switch (claimIdType.getTypeLabel()) { + case "fiss" -> ((RdaFissClaim) claimEntity).getMbiRecord(); + case "mcs" -> ((RdaMcsClaim) claimEntity).getMbiRecord(); + default -> throw new IllegalArgumentException( + "Invalid claim ID type '" + claimIdType.getTypeLabel() + "', cannot get claim data"); + }; + } + + /** + * Logs relevant identifiers (hash, ID) from a given {@link Mbi} to the MDC. + * + * @param mbi the {@link Mbi} to log + */ + private void logMbiIdentifiersToMdc(Mbi mbi) { + requireNonNull(mbi); + + BfdMDC.put(BfdMDC.MBI_HASH, mbi.getHash()); + BfdMDC.put(BfdMDC.MBI_ID, mbi.getMbiId().toString()); + } + /** * Implementation specific claim type parsing. * @@ -408,10 +450,6 @@ public Bundle findByPatient( new OffsetLinkBuilder( requestDetails, String.format("/%s?", resourceType.getSimpleName())); - if (isHashed) { - TransformerUtilsV2.logMbiHashToMdc(mbiString); - } - BundleOptions bundleOptions = new BundleOptions(isHashed, excludeSamhsa, includeTaxNumbers); if (types != null) { @@ -449,24 +487,40 @@ Bundle createBundleFor( DateRangeParam serviceDate, OffsetLinkBuilder paging, BundleOptions bundleOptions) { - List resources = new ArrayList<>(); - - for (ResourceTypeV2 type : resourceTypes) { - List entities; - - entities = - claimDao.findAllByMbiAttribute( - type, mbi, bundleOptions.isHashed, lastUpdated, serviceDate); - - resources.addAll( - entities.stream() - .filter(e -> !bundleOptions.excludeSamhsa || samhsaMatcher.hasNoSamhsaData(e)) - .map(e -> transformEntity(type, e, bundleOptions.includeTaxNumbers)) - .collect(Collectors.toList())); - } - - // Enforces a specific sorting for pagination that parities the EOB resource sorting. - resources.sort(Comparator.comparing(r -> r.getIdElement().getIdPart())); + var entitiesWithType = + resourceTypes.stream() + // There may be multiple resource types which will each result in a list of claim + // entities. So, to ensure that we have a flat list of entities to their type, we use + // flatMap + .flatMap( + type -> + claimDao + .findAllByMbiAttribute( + type, mbi, bundleOptions.isHashed, lastUpdated, serviceDate) + .stream() + .map(e -> new ImmutablePair<>(e, type))) + .toList(); + + // Log nonsensitive MBI identifiers for a given Claim/ClaimResponse request for use in + // historical analysis + entitiesWithType.stream() + .map(pair -> getClaimEntityMbi(pair.right, pair.left)) + .filter(Objects::nonNull) + // We choose the first MBI from the first, valid claim entity (technically, all entities + // should fit these criteria or something is very wrong) in the Stream as the MBI + // will be the same for all returned claims, so there is no reason to evaluate the entire + // Stream + .findFirst() + .ifPresent(this::logMbiIdentifiersToMdc); + + List resources = + entitiesWithType.stream() + .filter( + pair -> !bundleOptions.excludeSamhsa || samhsaMatcher.hasNoSamhsaData(pair.left)) + .map(pair -> transformEntity(pair.right, pair.left, bundleOptions.includeTaxNumbers)) + // Enforces a specific sorting for pagination that parities the EOB resource sorting. + .sorted(Comparator.comparing(r -> r.getIdElement().getIdPart())) + .collect(Collectors.toList()); Bundle bundle = new Bundle(); bundle.setTotal(resources.size()); diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/CoverageE2EBase.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/CoverageE2EBase.java index 0c75720a07..d35328d113 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/CoverageE2EBase.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/CoverageE2EBase.java @@ -389,7 +389,7 @@ public void testCoverageReadIdHasAccessJsonWithExpectedMdcKeys() throws IOExcept List additionalExpectedMdcKeys = List.of(BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_LOCATION); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys); } @@ -406,7 +406,7 @@ public void testCoverageSearchByBeneHasAccessJsonWithExpectedMdcKeys() throws IO List additionalExpectedMdcKeys = List.of(BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys); } diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ExplanationOfBenefitE2EBase.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ExplanationOfBenefitE2EBase.java index 2674e9e276..67b87c738d 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ExplanationOfBenefitE2EBase.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ExplanationOfBenefitE2EBase.java @@ -1013,7 +1013,7 @@ public void testEobReadIdHasAccessJsonWithExpectedMdcKeys() throws IOException { BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ADDRESS_FIELDS, BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ACCEPT_CHARSET); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys, headers); } @@ -1036,7 +1036,7 @@ public void testEobByPatientIdHasAccessJsonWithExpectedMdcKeys() throws IOExcept BfdMDC.HTTP_ACCESS_REQUEST_HEADER_TAX_NUMBERS, BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ADDRESS_FIELDS); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys, headers); } diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/PatientE2EBase.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/PatientE2EBase.java index 585116e8de..0d97e89c70 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/PatientE2EBase.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/PatientE2EBase.java @@ -834,7 +834,7 @@ public void testPatientByIdentifierHasAccessJsonWithExpectedMdcKeys() throws IOE List additionalExpectedMdcKeys = new ArrayList<>(); additionalExpectedMdcKeys.add(BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys); } @@ -850,7 +850,7 @@ public void testPatientReadHasAccessJsonWithExpectedMdcKeys() throws IOException List additionalExpectedMdcKeys = new ArrayList<>(); additionalExpectedMdcKeys.add(BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_LOCATION); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys); } @@ -866,7 +866,7 @@ public void testPatientByLogicalIdHasAccessJsonWithExpectedMdcKeys() throws IOEx List additionalExpectedMdcKeys = new ArrayList<>(); additionalExpectedMdcKeys.add(BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys); } diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ServerTestUtils.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ServerTestUtils.java index f1ff687efe..a4aa881d62 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ServerTestUtils.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/ServerTestUtils.java @@ -80,6 +80,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.management.MBeanServer; import javax.net.ssl.SSLContext; import javax.sql.DataSource; @@ -101,6 +102,41 @@ public final class ServerTestUtils { private static final Logger LOGGER = LoggerFactory.getLogger(ServerTestUtils.class); + /** The list of base mdc keys that should be on every write to access.json. */ + private static final List DEFAULT_MDC_KEYS = + Arrays.asList( + BfdMDC.BENE_ID, + BfdMDC.HAPI_RESPONSE_TIMESTAMP_MILLI, + BfdMDC.HAPI_POST_PROCESS_TIMESTAMP_MILLI, + BfdMDC.HAPI_PRE_HANDLE_TIMESTAMP_MILLI, + BfdMDC.HAPI_PRE_PROCESS_TIMESTAMP_MILLI, + BfdMDC.HAPI_PROCESSING_COMPLETED_TIMESTAMP_MILLI, + BfdMDC.HAPI_PROCESSING_COMPLETED_NORM_TIMESTAMP_MILLI, + BfdMDC.HTTP_ACCESS_REQUEST_CLIENTSSL_DN, + BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ACCEPT, + BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ACCEPT_ENCODING, + BfdMDC.HTTP_ACCESS_REQUEST_HEADER_CONN_ENCODING, + BfdMDC.HTTP_ACCESS_REQUEST_HEADER_HOST_ENCODING, + BfdMDC.HTTP_ACCESS_REQUEST_HEADER_USER_AGENT, + BfdMDC.HTTP_ACCESS_REQUEST_HTTP_METHOD, + BfdMDC.HTTP_ACCESS_REQUEST_OPERATION, + BfdMDC.HTTP_ACCESS_REQUEST_QUERY_STR, + BfdMDC.HTTP_ACCESS_REQUEST_TYPE, + BfdMDC.HTTP_ACCESS_REQUEST_URI, + BfdMDC.HTTP_ACCESS_REQUEST_URL, + BfdMDC.HTTP_ACCESS_REQUEST_HEADER_CONTENT_TYPE, + BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_MILLISECONDS, + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_ENCODING, + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_TYPE, + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_DATE, + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_LAST_MODIFIED, + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_POWERED_BY, + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_REQUEST_ID, + BfdMDC.HTTP_ACCESS_RESPONSE_OUTPUT_SIZE_IN_BYTES, + BfdMDC.HTTP_ACCESS_RESPONSE_STATUS, + BfdMDC.HTTP_ACCESS_RESPONSE_CONTENT_LENGTH, + BfdMDC.RESOURCES_RETURNED); + /** The singleton {@link ServerTestUtils} instance to use everywhere. */ private static ServerTestUtils SINGLETON; @@ -787,9 +823,9 @@ public static void writeFile(String contents, Path filePath) { } /** - * Assert mdc keys are written to the access.json log when calling the given REST query string. - * Will check a set of common mdc keys that should be on every call, and optionally takes a list - * of additional keys to check for specific to the request made. + * Assert mdc keys (not values) are written to the access.json log when calling the given REST + * query string. Will check a set of common mdc keys that should be on every call, and optionally + * takes a list of additional keys to check for specific to the request made. * * @param requestAuth the request auth for the restAssured call * @param requestString the request string for the restAssured call @@ -797,17 +833,17 @@ public static void writeFile(String contents, Path filePath) { * mdc keys to check for in the access.json * @throws IOException if there is an issue with the IO around the access.json file */ - public static void assertAccessJsonHasMdcKeys( + public static void assertDefaultAndAdditionalMdcKeys( RequestSpecification requestAuth, String requestString, List additionalMdcKeysToCheck) throws IOException { - assertAccessJsonHasMdcKeys( + assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalMdcKeysToCheck, new HashMap<>()); } /** - * Assert mdc keys are written to the access.json log when calling the given REST query string. - * Will check a set of common mdc keys that should be on every call, and optionally takes a list - * of additional keys to check for specific to the request made. + * Assert mdc keys (not values) are written to the access.json log when calling the given REST + * query string. Will check a set of common mdc keys that should be on every call, and optionally + * takes a list of additional keys to check for specific to the request made. * * @param requestAuth the request auth for the restAssured call * @param requestString the request string for the restAssured call @@ -816,61 +852,68 @@ public static void assertAccessJsonHasMdcKeys( * @param headers the headers needed for the restAssured request * @throws IOException if there is an issue with the IO around the access.json file */ - public static void assertAccessJsonHasMdcKeys( + public static void assertDefaultAndAdditionalMdcKeys( RequestSpecification requestAuth, String requestString, List additionalMdcKeysToCheck, Map headers) throws IOException { + assertMdcEntries( + requestAuth, + requestString, + // Generate a Map that is the combination of the default keys and any additional keys to + // check where each key has a corresponding empty value indicating that only the key's + // existence should be verified; basically, this method doesn't check any MDC values + Stream.concat(DEFAULT_MDC_KEYS.stream(), additionalMdcKeysToCheck.stream()) + .distinct() + .collect(Collectors.toMap(key -> key, key -> Optional.empty())), + headers); + } + + /** + * Assert that MDC entries provided exist in the access.jon log, and optionally check the values + * of the entries if the value provided for a given MDC key is not an empty {@link Optional} + * value. + * + * @param requestAuth the request auth for the restAssured call + * @param requestString the request string for the restAssured call + * @param expectedMdcKeysToValues a {@link Map} of MDC keys to values that are asserted upon. Keys + * are always checked for existence, whereas values are only checked if a non-empty value is + * provided + * @throws IOException if there is an issue with the IO around the access.json file + */ + public static void assertMdcEntries( + RequestSpecification requestAuth, + String requestString, + Map> expectedMdcKeysToValues) + throws IOException { + assertMdcEntries(requestAuth, requestString, expectedMdcKeysToValues, new HashMap<>()); + } + /** + * Assert that MDC entries provided exist in the access.jon log, and optionally check the values + * of the entries if the value provided for a given MDC key is not an empty {@link Optional} + * value. + * + * @param requestAuth the request auth for the restAssured call + * @param requestString the request string for the restAssured call + * @param expectedMdcKeysToValues a {@link Map} of MDC keys to values that are asserted upon. Keys + * are always checked for existence, whereas values are only checked if a non-empty value is + * provided + * @param headers the headers needed for the restAssured request + * @throws IOException if there is an issue with the IO around the access.json file + */ + public static void assertMdcEntries( + RequestSpecification requestAuth, + String requestString, + Map> expectedMdcKeysToValues, + Map headers) + throws IOException { Path accessLogJson = ServerTestUtils.getWarProjectDirectory() .resolve("target") .resolve("server-work") .resolve("access.json"); - - // A list of base mdc keys that should be on every write to access.json - List baseKeysToCheck = - Arrays.asList( - BfdMDC.BENE_ID, - BfdMDC.HAPI_RESPONSE_TIMESTAMP_MILLI, - BfdMDC.HAPI_POST_PROCESS_TIMESTAMP_MILLI, - BfdMDC.HAPI_PRE_HANDLE_TIMESTAMP_MILLI, - BfdMDC.HAPI_PRE_PROCESS_TIMESTAMP_MILLI, - BfdMDC.HAPI_PROCESSING_COMPLETED_TIMESTAMP_MILLI, - BfdMDC.HAPI_PROCESSING_COMPLETED_NORM_TIMESTAMP_MILLI, - BfdMDC.HTTP_ACCESS_REQUEST_CLIENTSSL_DN, - BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ACCEPT, - BfdMDC.HTTP_ACCESS_REQUEST_HEADER_ACCEPT_ENCODING, - BfdMDC.HTTP_ACCESS_REQUEST_HEADER_CONN_ENCODING, - BfdMDC.HTTP_ACCESS_REQUEST_HEADER_HOST_ENCODING, - BfdMDC.HTTP_ACCESS_REQUEST_HEADER_USER_AGENT, - BfdMDC.HTTP_ACCESS_REQUEST_HTTP_METHOD, - BfdMDC.HTTP_ACCESS_REQUEST_OPERATION, - BfdMDC.HTTP_ACCESS_REQUEST_QUERY_STR, - BfdMDC.HTTP_ACCESS_REQUEST_TYPE, - BfdMDC.HTTP_ACCESS_REQUEST_URI, - BfdMDC.HTTP_ACCESS_REQUEST_URL, - BfdMDC.HTTP_ACCESS_REQUEST_HEADER_CONTENT_TYPE, - BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_MILLISECONDS, - BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_ENCODING, - BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_TYPE, - BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_DATE, - BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_LAST_MODIFIED, - BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_POWERED_BY, - BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_REQUEST_ID, - BfdMDC.HTTP_ACCESS_RESPONSE_OUTPUT_SIZE_IN_BYTES, - BfdMDC.HTTP_ACCESS_RESPONSE_STATUS, - BfdMDC.HTTP_ACCESS_RESPONSE_CONTENT_LENGTH, - BfdMDC.RESOURCES_RETURNED); - - // Copy array so its mutable for the below add - List mdcAccessKeysToCheck = new ArrayList<>(baseKeysToCheck); - - // If there are any other additional mdc keys specific to this call, add them to the list to - // check - mdcAccessKeysToCheck.addAll(additionalMdcKeysToCheck); - // Empty the access json to avoid pollution from other tests try (BufferedWriter writer = Files.newBufferedWriter(accessLogJson)) { Files.writeString(accessLogJson, ""); @@ -906,7 +949,7 @@ public static void assertAccessJsonHasMdcKeys( // Compile a list of mdc keys that are missing from the expected list List missingMdcKeys = new ArrayList<>(); - for (String mdcKey : mdcAccessKeysToCheck) { + for (String mdcKey : expectedMdcKeysToValues.keySet()) { if (!mdcKeyValueMap.containsKey(mdcKey)) { LOGGER.info("Missing header: " + mdcKey); missingMdcKeys.add(mdcKey); @@ -918,16 +961,41 @@ public static void assertAccessJsonHasMdcKeys( * testing them on all endpoints */ List extraMdcKeys = new ArrayList<>(); for (String mdcKey : mdcKeyValueMap.keySet()) { - if (!mdcAccessKeysToCheck.contains(mdcKey) + if (!expectedMdcKeysToValues.containsKey(mdcKey) + && !DEFAULT_MDC_KEYS.contains(mdcKey) && !mdcKey.startsWith("jpa") && !mdcKey.startsWith("database")) { LOGGER.info("Extra header: " + mdcKey); extraMdcKeys.add(mdcKey); } } + + // Reduce the expected MDC entries to a subset of entries with values we'd like to check; that + // is, any entry with a value that isn't an empty Optional + Map mdcEntriesWithExpectedVals = + expectedMdcKeysToValues.entrySet().stream() + .filter(es -> es.getValue().isPresent()) + .map(es -> Map.entry(es.getKey(), es.getValue().get())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + + // Reduce the actual values map to one with keys matching that of the expected, checked map + // above. This is necessary as we only want to check the value of a particular entry if a value + // was provided + Map actualMdcEntriesToCheck = + mdcKeyValueMap.entrySet().stream() + .filter(es -> mdcEntriesWithExpectedVals.containsKey(es.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + // Check we have no missing mdc keys in a way that makes a useful assertion error if not assertEquals(missingMdcKeys, new ArrayList<>(), "Missing expected MDC keys in access.json"); assertEquals( extraMdcKeys, new ArrayList<>(), "Found unexpected additional MDC keys in access.json"); + + // Check the MDC entries with an expected value against the real value assertMdcEntries from + // access.json + assertEquals( + mdcEntriesWithExpectedVals, + actualMdcEntriesToCheck, + "Found MDC entries with unexpected values in access.json"); } } diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/PatientE2E.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/PatientE2E.java index 6d27ace22d..d98ea037dd 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/PatientE2E.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/PatientE2E.java @@ -217,7 +217,7 @@ public void testPatientByPartDContractHasAccessJsonWithExpectedMdcKeys() throws List additionalExpectedMdcKeys = new ArrayList<>(MDC_EXPECTED_BASE_KEYS); additionalExpectedMdcKeys.add(BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys, headers); } diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimE2E.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimE2E.java index 017a861b89..9bcc8b3310 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimE2E.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimE2E.java @@ -4,12 +4,18 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; +import gov.cms.bfd.model.rda.Mbi; +import gov.cms.bfd.server.sharedutils.BfdMDC; import gov.cms.bfd.server.war.ServerRequiredTest; +import gov.cms.bfd.server.war.ServerTestUtils; import gov.cms.bfd.server.war.commons.CommonHeaders; import gov.cms.bfd.server.war.utils.AssertUtils; import gov.cms.bfd.server.war.utils.RDATestUtils; +import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.Set; import org.hl7.fhir.r4.model.Claim; import org.junit.jupiter.api.AfterAll; @@ -152,6 +158,52 @@ public void shouldGetCorrectMcsClaimResourceByIdWithTaxNumbers() { requestString, true, "claimMcsReadWithTaxNumbers", READ_IGNORE_PATTERNS); } + /** + * Tests to see if nonsensitive MBI identifiers (hash and ID) are logged to the MDC when a FISS + * {@link Claim} with an associated {@link Mbi} is looked up by a specific ID. + */ + @Test + void testClaimReadLogsMbiIdentifiersForFissClaimWithMbiRecord() throws IOException { + String requestString = claimEndpoint + "f-123456"; + + Mbi testingMbi = rdaTestUtils.lookupTestMbiRecord(rdaTestUtils.getEntityManager()); + ServerTestUtils.assertMdcEntries( + requestAuth, + requestString, + Map.of( + BfdMDC.MBI_HASH, + Optional.of(testingMbi.getHash()), + BfdMDC.MBI_ID, + Optional.of(testingMbi.getMbiId().toString()), + // This isn't a default key, but we need to include it without checking its value to + // satisfy the assertion for extra, unexpected MDC keys + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_LOCATION, + Optional.empty())); + } + + /** + * Tests to see if nonsensitive MBI identifiers (hash and ID) are logged to the MDC when an MCS + * {@link Claim} with an associated {@link Mbi} is looked up by a specific ID. + */ + @Test + void testClaimReadLogsMbiIdentifiersForMcsClaimWithMbiRecord() throws IOException { + String requestString = claimEndpoint + "m-654321"; + + Mbi testingMbi = rdaTestUtils.lookupTestMbiRecord(rdaTestUtils.getEntityManager()); + ServerTestUtils.assertMdcEntries( + requestAuth, + requestString, + Map.of( + BfdMDC.MBI_HASH, + Optional.of(testingMbi.getHash()), + BfdMDC.MBI_ID, + Optional.of(testingMbi.getMbiId().toString()), + // This isn't a default key, but we need to include it without checking its value to + // satisfy the assertion for extra, unexpected MDC keys + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_LOCATION, + Optional.empty())); + } + /** * Tests to see if the correct response is given when a search is done for {@link Claim}s using * given mbi and service-date range. In this test case the query finds the matched claims because @@ -267,6 +319,29 @@ public void testClaimFindByPatientWithPagingStartOneLessThanMaxExpect200() { .get(requestString); } + /** + * Verify that Claim logs nonsensitive MBI identifiers (hash and ID) to the MDC log when receiving + * a valid Claim request. + */ + @Test + public void testClaimFindByPatientLogsMbiIdentifiers() throws IOException { + String requestString = claimEndpoint + "?mbi=" + RDATestUtils.MBI_HASH; + + Mbi testingMbi = rdaTestUtils.lookupTestMbiRecord(rdaTestUtils.getEntityManager()); + ServerTestUtils.assertMdcEntries( + requestAuth, + requestString, + Map.of( + BfdMDC.MBI_HASH, + Optional.of(testingMbi.getHash()), + BfdMDC.MBI_ID, + Optional.of(testingMbi.getMbiId().toString()), + // This isn't a default key, but we need to include it without checking its value to + // satisfy the assertion for extra, unexpected MDC keys + BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB, + Optional.empty())); + } + /** * Verify that an empty bundle is returned when pagination is requested but no results are * returned. Normally this would return a 400 since the default startIndex is equal to the number diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimResponseE2E.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimResponseE2E.java index 9747539663..af666fa30d 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimResponseE2E.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/r4/providers/pac/ClaimResponseE2E.java @@ -4,9 +4,15 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; +import gov.cms.bfd.model.rda.Mbi; +import gov.cms.bfd.server.sharedutils.BfdMDC; import gov.cms.bfd.server.war.ServerRequiredTest; +import gov.cms.bfd.server.war.ServerTestUtils; import gov.cms.bfd.server.war.utils.AssertUtils; import gov.cms.bfd.server.war.utils.RDATestUtils; +import java.io.IOException; +import java.util.Map; +import java.util.Optional; import java.util.Set; import org.hl7.fhir.r4.model.ClaimResponse; import org.junit.jupiter.api.AfterAll; @@ -75,6 +81,52 @@ void shouldGetCorrectMcsClaimResponseResourceById() { verifyResponseMatchesFor(requestString, "claimResponseMcsRead", READ_IGNORE_PATTERNS); } + /** + * Tests to see if nonsensitive MBI identifiers (hash and ID) are logged to the MDC when a FISS + * {@link ClaimResponse} with an associated {@link Mbi} is looked up by a specific ID. + */ + @Test + void testClaimResponseReadLogsMbiIdentifiersForFissClaimWithMbiRecord() throws IOException { + String requestString = claimResponseEndpoint + "f-123456"; + + Mbi testingMbi = rdaTestUtils.lookupTestMbiRecord(rdaTestUtils.getEntityManager()); + ServerTestUtils.assertMdcEntries( + requestAuth, + requestString, + Map.of( + BfdMDC.MBI_HASH, + Optional.of(testingMbi.getHash()), + BfdMDC.MBI_ID, + Optional.of(testingMbi.getMbiId().toString()), + // This isn't a default key, but we need to include it without checking its value to + // satisfy the assertion for extra, unexpected MDC keys + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_LOCATION, + Optional.empty())); + } + + /** + * Tests to see if nonsensitive MBI identifiers (hash and ID) are logged to the MDC when an MCS + * {@link ClaimResponse} with an associated {@link Mbi} is looked up by a specific ID. + */ + @Test + void testClaimResponseReadLogsMbiIdentifiersForMcsClaimWithMbiRecord() throws IOException { + String requestString = claimResponseEndpoint + "m-654321"; + + Mbi testingMbi = rdaTestUtils.lookupTestMbiRecord(rdaTestUtils.getEntityManager()); + ServerTestUtils.assertMdcEntries( + requestAuth, + requestString, + Map.of( + BfdMDC.MBI_HASH, + Optional.of(testingMbi.getHash()), + BfdMDC.MBI_ID, + Optional.of(testingMbi.getMbiId().toString()), + // This isn't a default key, but we need to include it without checking its value to + // satisfy the assertion for extra, unexpected MDC keys + BfdMDC.HTTP_ACCESS_RESPONSE_HEADER_CONTENT_LOCATION, + Optional.empty())); + } + /** * Tests to see if the correct response is given when a search is done for {@link ClaimResponse}s * using given mbi and service-date range. In this test case the query finds the matched claims @@ -199,6 +251,29 @@ public void testClaimResponseFindByPatientWithPagingStartOneLessThanMaxExpect200 .get(requestString); } + /** + * Verify that ClaimResponse logs nonsensitive MBI identifiers (hash and ID) to the MDC log when + * receiving a valid ClaimResponse request. + */ + @Test + public void testClaimResponseFindByPatientLogsMbiIdentifiers() throws IOException { + String requestString = claimResponseEndpoint + "?mbi=" + RDATestUtils.MBI_HASH; + + Mbi testingMbi = rdaTestUtils.lookupTestMbiRecord(rdaTestUtils.getEntityManager()); + ServerTestUtils.assertMdcEntries( + requestAuth, + requestString, + Map.of( + BfdMDC.MBI_HASH, + Optional.of(testingMbi.getHash()), + BfdMDC.MBI_ID, + Optional.of(testingMbi.getMbiId().toString()), + // This isn't a default key, but we need to include it without checking its value to + // satisfy the assertion for extra, unexpected MDC keys + BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB, + Optional.empty())); + } + /** * Verify that an empty bundle is returned when pagination is requested but no results are * returned. Normally this would return a 400 since the default startIndex is equal to the number diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/stu3/providers/PatientE2E.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/stu3/providers/PatientE2E.java index 60ad485117..30e17434dc 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/stu3/providers/PatientE2E.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/stu3/providers/PatientE2E.java @@ -705,7 +705,7 @@ public void testPatientByPartDContractHasAccessJsonWithExpectedMdcKeys() throws additionalExpectedMdcKeys.add(BfdMDC.HTTP_ACCESS_RESPONSE_DURATION_PER_KB); additionalExpectedMdcKeys.add(BfdMDC.HTTP_ACCESS_REQUEST_HEADER_IDENTIFIERS); - ServerTestUtils.assertAccessJsonHasMdcKeys( + ServerTestUtils.assertDefaultAndAdditionalMdcKeys( requestAuth, requestString, additionalExpectedMdcKeys, headers); } diff --git a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/utils/RDATestUtils.java b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/utils/RDATestUtils.java index f0e520baeb..6eb0c1e9ba 100644 --- a/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/utils/RDATestUtils.java +++ b/apps/bfd-server/bfd-server-war/src/test/java/gov/cms/bfd/server/war/utils/RDATestUtils.java @@ -690,7 +690,7 @@ public void seedMcsClaimForServiceIdTest( * @param em our {@link EntityManager} * @return the {@link Mbi} */ - private Mbi lookupTestMbiRecord(EntityManager em) { + public Mbi lookupTestMbiRecord(EntityManager em) { final CriteriaBuilder builder = em.getCriteriaBuilder(); final CriteriaQuery criteria = builder.createQuery(Mbi.class); final Root root = criteria.from(Mbi.class); diff --git a/ops/terraform/services/server/insights/api-requests/glue.tf b/ops/terraform/services/server/insights/api-requests/glue.tf index 47bbfba6aa..5f337a7345 100644 --- a/ops/terraform/services/server/insights/api-requests/glue.tf +++ b/ops/terraform/services/server/insights/api-requests/glue.tf @@ -1373,6 +1373,17 @@ module "glue-table-api-requests" { "type" = "string", "comment" = "" }, + { + "name" = "mdc_mbi_id", + "type" = "string", + "comment" = "" + }, + + { + "name" = "mdc_mbi_hash", + "type" = "string", + "comment" = "" + } ] }