MockClient returns incorrect data when searching with token parameters due to the way matchesTokenFilter operates

[pamella]

Issue

MockClient returns incorrect data when searching with token parameters because the current implementation of the matchesTokenFilter function does not support exact match searches, as specified in the FHIR documentation.

Problem

In the original implementation, the matchesTokenFilter function uses the includes method to check if the token value is included in the search parameter. This implementation does not support exact match searches, as specified in the FHIR documentation.

Taking into account that the MockClient uses the MemoryRepository which uses the matchesSearchRequest function to filter the resources, we can get incorrect results when searching with token parameters. For example, if mockClient.upsertResource is called with an observation with code 12345-6 and then another mockClient.upsertResource is called with an observation with code 1234, the first will be replaced by the second one because MemoryRepository will be used and it will consider they are the same resource because the matchesSearchRequest function will return true when it should return false.

Examples

Example 1

When searching for a patient with the identifier value 1234567890, the matchesTokenFilter function will return true for any identifier value that includes 1234567890, such as 1234, 12345, etc.

When we add the following test to packages/core/src/search/match.test.ts, it fails when it should pass:

➡️ Click to expand and see the code

test('[Token] Datatype: Identifier', () => {
  const identifier = '1234567890';
  const identifierSubstring = identifier.substring(0, 4);
  const resource: Patient = {
    resourceType: 'Patient',
    identifier: [
      {
        system: 'http:https://example.com',
        value: identifier,
      },
      {
        system: 'http:https://test.com',
        value: identifier,
      },
      {
        system: 'http:https://test.com',
        value: 'foo',
      },
    ],
  };

  expect(
    matchesSearchRequest(resource, {
      resourceType: 'Patient',
      filters: [
        { code: 'identifier', operator: Operator.EQUALS, value: 'http:https://example.com|' + identifierSubstring },
      ],
    })
  ).toBe(false); // FAIL! It currently returns true, but it should return false

  expect(
    matchesSearchRequest(resource, {
      resourceType: 'Patient',
      filters: [{ code: 'identifier', operator: Operator.EQUALS, value: identifierSubstring }],
    })
  ).toBe(false); // FAIL! It currently returns true, but it should return false
});

Example 2

When searching for an observation with the code value 12345-6, the matchesTokenFilter function will return true for any code value that includes 12345-6, such as 12345, 12345-, etc.

When we add the following test to packages/core/src/search/match.test.ts, it fails when it should pass:

➡️ Click to expand and see the code

test('[Token] Datatype: CodeableConcept', () => {
  const identifier = '12345-6';
  const identifierSubstring = identifier.substring(0, 4);
  const resource: Observation = {
    resourceType: 'Observation',
    status: 'final',
    code: {
      coding: [
        {
          system: 'http:https://example.com',
          code: identifier,
        },
        {
          system: 'http:https://test.com',
          code: identifier,
        },
        {
          system: 'http:https://test.com',
          code: 'foo',
        },
      ],
      text: 'test',
    },
  };

  expect(
    matchesSearchRequest(resource, {
      resourceType: 'Observation',
      filters: [{ code: 'code', operator: Operator.EQUALS, value: 'http:https://example.com|' + identifierSubstring }],
    })
  ).toBe(false); // FAIL! It currently returns true, but it should return false

  expect(
    matchesSearchRequest(resource, {
      resourceType: 'Observation',
      filters: [{ code: 'code', operator: Operator.EQUALS, value: identifierSubstring }],
    })
  ).toBe(false); // FAIL! It currently returns true, but it should return false
});

Proposed solution

That being said, I thought about a partial solution that would allow us to support exact match searches for token parameters. I say partial because it would address the identifier and code types. Here is a code snippet:

➡️ Click to expand and see the code

function matchesStringFilter(
  resource: Resource,
  filter: Filter,
  searchParam: SearchParameter,
  asToken?: boolean
): boolean {
  const details = getSearchParameterDetails(resource.resourceType, searchParam);
  const searchParamElementType = details.elementDefinitions?.[0].type?.[0].code;
  const resourceValues = evalFhirPath(searchParam.expression as string, resource);
  const filterValues = filter.value.split(',');
  const negated = isNegated(filter.operator);
  for (const resourceValue of resourceValues) {
    for (const filterValue of filterValues) {
      let match;
      if (searchParamElementType === PropertyType.Identifier) {
        match = matchesTokenIdentifierValue(resourceValue as Identifier, filter.operator, filterValue);
      } else if (searchParamElementType === PropertyType.CodeableConcept) {
        match = matchesTokenCodeableConceptValue(resourceValue as Coding, filter.operator, filterValue);
      } else {
        match = matchesStringValue(resourceValue, filter.operator, filterValue, asToken);
      }
      if (match) {
        return !negated;
      }
    }
  }
  // If "not equals" and no matches, then return true
  // If "equals" and no matches, then return false
  return negated;
}

function matchesTokenCodeableConceptValue(
  resourceValue: CodeableConcept,
  operator: Operator,
  filterValue: string
): boolean {
  if (filterValue.includes('|')) {
    const [system, code] = filterValue.split('|');
    return (
      resourceValue.coding?.some(
        (coding) =>
          coding.system?.toLowerCase() === system.toLowerCase() && coding.code?.toLowerCase() === code.toLowerCase()
      ) ?? false
    );
  }

  return (
    resourceValue.text?.toLowerCase() === filterValue.toLowerCase() ||
    (resourceValue.coding?.some((coding) => coding.code?.toLowerCase() === filterValue.toLowerCase()) ?? false)
  );
}

I was working on this solution, however I wonder if:

• It would be enough to have a partial solution because I understand there are a lot of nuances in FHIR standards about search such as operators and modifiers.
• It would be a overkill to implement a full solution considering (I suppose) that the main usage is for testing purposes.
• It might be a breaking change to other matches* functions that are using the matchesStringFilter function. For example, matchesAccessPolicyResourcePolicy and matchesCriteria.
• It might be a breaking change to customers that are already using the MockClient class.

So I would like to know your opinion about all of this.

Please let me know if you have any questions or need further information. I am looking forward to your feedback. Thank you!

[rahul1]

Thanks @pamella . This is related to #1237

[codyebberson]

Thanks @pamella - I added comments to your draft PR. This all looks good to me 👍 Thank you 🙏

[rahul1]

Thanks @pamella