Skip to content

Commit

Permalink
Relaxes split_if_relative_reference validation constraints in C++ a…
Browse files Browse the repository at this point in the history
…nd Python.

See also: https://www.hl7.org/fhir/references.html#literal.

Closes #24.

PiperOrigin-RevId: 345316065
  • Loading branch information
Cameron Tew authored and nickgeorge committed Dec 3, 2020
1 parent 4495912 commit 4df1d5d
Show file tree
Hide file tree
Showing 12 changed files with 40 additions and 47 deletions.
1 change: 1 addition & 0 deletions cc/google/fhir/json_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "google/fhir/status/statusor.h"
#include "google/fhir/stu3/profiles.h"
#include "google/fhir/util.h"
#include "google/fhir/references.h"
#include "proto/google/fhir/proto/annotations.pb.h"
#include "include/json/json.h"
#include "re2/re2.h"
Expand Down
10 changes: 2 additions & 8 deletions cc/google/fhir/references.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,8 @@ absl::Status SplitIfRelativeReference(Message* reference) {
return absl::OkStatus();
}

// We're permissive about various full url schemes.
static LazyRE2 kUrlReference = {"(http|https|urn):.*"};
if (RE2::FullMatch(uri_string, *kUrlReference)) {
// There's no way to rewrite the URI, but it's valid as is.
return absl::OkStatus();
}
return InvalidArgumentError(absl::StrCat(
"String \"", uri_string, "\" cannot be parsed as a reference."));
// There's no way to rewrite the URI, but it's valid as is.
return absl::OkStatus();
}

namespace internal {
Expand Down
19 changes: 19 additions & 0 deletions cc/google/fhir/references.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,25 @@ absl::StatusOr<const ::google::protobuf::FieldDescriptor*> GetReferenceFieldForR

} // namespace internal

// If possible, parses a reference `uri` field into more structured fields.
// This is only possible for two forms of reference uris:
// * Relative references of the form $TYPE/$ID, e.g., "Patient/1234"
// In this case, this will be parsed to a proto of the form:
// {patient_id: {value: "1234"}}
// * Fragments of the form "#$FRAGMENT", e.g., "#vs1". In this case, this would
// be parsed into a proto of the form:
// {fragment: {value: "vs1"} }
//
// If the reference URI matches one of these schemas, the `uri` field will be
// cleared, and the appropriate structured fields set.
//
// If it does not match any of these schemas, the reference will be unchanged,
// and an OK status will be returned.
//
// If the message is not a valid FHIR Reference proto, this will return a
// failure status.
absl::Status SplitIfRelativeReference(::google::protobuf::Message* reference);

// Return the full string representation of a reference.
absl::StatusOr<std::string> ReferenceProtoToString(
const ::google::protobuf::Message& reference);
Expand Down
1 change: 1 addition & 0 deletions cc/google/fhir/testutil/generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "google/fhir/annotations.h"
#include "google/fhir/fhir_types.h"
#include "google/fhir/proto_util.h"
#include "google/fhir/references.h"
#include "google/fhir/util.h"
#include "include/json/json.h"

Expand Down
4 changes: 0 additions & 4 deletions cc/google/fhir/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@
namespace google {
namespace fhir {

// Splits relative references into their components, for example, "Patient/ABCD"
// will result in the patientId field getting the value "ABCD".
absl::Status SplitIfRelativeReference(::google::protobuf::Message* reference);

// Builds an absl::Time from a time-like fhir Element.
// Must have a value_us field.
template <class T>
Expand Down
9 changes: 0 additions & 9 deletions py/google/fhir/r4/references_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,5 @@ def testSplitIfRelativeReference_withUrlScheme_succeeds(self):
datatypes_pb2.String(
value='http://acme.com/ehr/fhir/Practitioner/2323-33-4'))

def testSplitIfRelativeReference_withMalformedUri_raises(self):
ref = datatypes_pb2.Reference(uri=datatypes_pb2.String(value='InvalidUri'))

with self.assertRaises(ValueError) as e:
references.split_if_relative_reference(ref)

self.assertIsInstance(e.exception, ValueError)


if __name__ == '__main__':
absltest.main()
27 changes: 15 additions & 12 deletions py/google/fhir/references.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
_INTERNAL_REFERENCE_PATTERN = re.compile(
r'^(?P<resource_type>[0-9A-Za-z_]+)/(?P<resource_id>[A-Za-z0-9.-]{1,64})'
r'(?:/_history/(?P<version>[A-Za-z0-9.-]{1,64}))?$')
_URL_REFERENCE_PATTERN = re.compile(r'^(http|https|urn):.*$')


def _validate_reference(reference: message.Message):
Expand Down Expand Up @@ -68,13 +67,25 @@ def populate_typed_reference_id(reference_id: message.Message, resource_id: str,


def split_if_relative_reference(reference: message.Message):
"""Splits relative references into their components.
"""If possible, parses a `Reference` `uri` into more structured fields.
For example, a reference with the uri set to "Patient/ABCD" will be changed
into a reference with the patientId set to "ABCD".
This is only possible for two forms of reference uris:
* Relative references of the form $TYPE/$ID, e.g., "Patient/1234"
In this case, this will be parsed to a proto of the form:
{patient_id: {value: "1234"}}
* Fragments of the form "#$FRAGMENT", e.g., "#vs1". In this case, this would
be parsed into a proto of the form:
{fragment: {value: "vs1"} }
If the reference URI matches one of these schemas, the `uri` field will be
cleared, and the appropriate structured fields set. Otherwise, the reference
will be unchanged.
Args:
reference: The FHIR reference to potentially split.
Raises:
ValueError: If the message is not a valid FHIR Reference proto.
"""
_validate_reference(reference)
uri_field = reference.DESCRIPTOR.fields_by_name.get('uri')
Expand All @@ -83,12 +94,6 @@ def split_if_relative_reference(reference: message.Message):

uri = proto_utils.get_value_at_field(reference, uri_field)

# We're permissive about various full URL schemes. If we're able to find a
# match, the URI is valid as-is.
url_match = re.fullmatch(_URL_REFERENCE_PATTERN, uri.value)
if url_match is not None:
return # URI is valid

internal_match = re.fullmatch(_INTERNAL_REFERENCE_PATTERN, uri.value)
if internal_match is not None:
# Note that we make the reference_id off of the reference before adding it,
Expand Down Expand Up @@ -125,8 +130,6 @@ def split_if_relative_reference(reference: message.Message):
proto_utils.set_value_at_field(reference, fragment_field, fragment)
return

raise ValueError(f'String {uri.value!r} cannot be parsed as a reference.')


def reference_to_string(reference: message.Message) -> str:
"""Returns a reference URI for a typed reference message."""
Expand Down
8 changes: 0 additions & 8 deletions py/google/fhir/stu3/references_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,6 @@ def testSplitIfRelativeReference_withUrlScheme_succeeds(self):
datatypes_pb2.String(
value='http://acme.com/ehr/fhir/Practitioner/2323-33-4'))

def testSplitIfRelativeReference_withMalformedUri_raises(self):
ref = datatypes_pb2.Reference(uri=datatypes_pb2.String(value='InvalidUri'))

with self.assertRaises(ValueError) as e:
references.split_if_relative_reference(ref)

self.assertIsInstance(e.exception, ValueError)


if __name__ == '__main__':
absltest.main()
3 changes: 0 additions & 3 deletions testdata/r4/validation/reference.invalid.ndjson
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
{"reference": "Patient/spaces not allowed"}
{"reference": "Patient/\"notallowed"}
{"reference": "Unknown/resourceType"}
{"reference": "#spaces not allowed"}
{"uri": "invalidField"}
{"patient_id": "unknownField"}
{"patientId": "unknownField"}
1 change: 1 addition & 0 deletions testdata/r4/validation/reference.valid.ndjson
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
{"reference": "FamilyMemberHistory/1"}
{"reference": "urn:uuid:49a86246-4004-42eb-9bdc-f542f93f9228"}
{"display": "Display Field Only"}
{"reference": "Practitioner?identifier=http://hl7.org/fhir/sid/us-npi|9999999959"}
3 changes: 0 additions & 3 deletions testdata/stu3/validation/reference.invalid.ndjson
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
{"reference": "Patient/spaces not allowed"}
{"reference": "Patient/\"notallowed"}
{"reference": "Unknown/resourceType"}
{"reference": "#spaces not allowed"}
{"uri": "invalidField"}
{"patient_id": "unknownField"}
{"patientId": "unknownField"}
1 change: 1 addition & 0 deletions testdata/stu3/validation/reference.valid.ndjson
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
{"reference": "FamilyMemberHistory/1"}
{"reference": "urn:uuid:49a86246-4004-42eb-9bdc-f542f93f9228"}
{"display": "Display Field Only"}
{"reference": "Practitioner?identifier=http://hl7.org/fhir/sid/us-npi|9999999959"}

0 comments on commit 4df1d5d

Please sign in to comment.