When a ContainedResource Message is passed to FhirPathValidator::Vali…

…date, extract the resource from the container before validating.

Without doing this, ContainedResource, an implementation detail, shows up in the constraint path and node path in the validation result.

PiperOrigin-RevId: 366531247

cc/google/fhir/fhir_path/BUILD

@@ -74,6 +74,7 @@ cc_library(
  "//cc/google/fhir:annotations",
  "//cc/google/fhir:primitive_handler",
  "//cc/google/fhir:proto_util",
+ "//cc/google/fhir:util",
  "//cc/google/fhir/status:statusor",
  "//proto/google/fhir/proto:annotations_cc_proto",
  "@com_google_absl//absl/base:core_headers",
@@ -228,6 +229,7 @@ cc_test(
  ":fhir_path_validation",
  ":r4_fhir_path_validation",
  ":stu3_fhir_path_validation",
+ "//cc/google/fhir/status",
  "//cc/google/fhir/status:statusor",
  "//cc/google/fhir/testutil:fhir_test_env",
  "//proto/google/fhir/proto/r4:uscore_cc_proto",
@@ -246,6 +248,7 @@ cc_test(
  "//proto/google/fhir/proto/stu3:uscore_cc_proto",
  "//proto/google/fhir/proto/stu3:uscore_codes_cc_proto",
  "@com_google_absl//absl/status",
+ "@com_google_absl//absl/status:statusor",
  "@com_google_googletest//:gtest_main",
  "@com_google_protobuf//:protobuf",
  ],

cc/google/fhir/fhir_path/fhir_path_validation.cc

@@ -28,6 +28,7 @@
 #include "google/fhir/fhir_path/fhir_path.h"
 #include "google/fhir/proto_util.h"
 #include "google/fhir/status/statusor.h"
+#include "google/fhir/util.h"
 #include "proto/google/fhir/proto/annotations.pb.h"
 
 namespace google {
@@ -250,8 +251,17 @@ absl::Status ValidationResults::LegacyValidationResult() const {
  ": \"", (*result).Constraint(), "\""));
 }
 
-ValidationResults FhirPathValidator::Validate(
- const ::google::protobuf::Message& message) {
+absl::StatusOr<ValidationResults> FhirPathValidator::Validate(
+ const Message& message) {
+ // ContainedResource is an implementation detail of FHIR protos. Extract the
+ // resource from the wrapper before prcoessing so that wrapper is not included
+ // in the node/constraint path of the validation results.
+ if (IsContainedResource(message)) {
+ FHIR_ASSIGN_OR_RETURN(const Message* resource,
+ GetContainedResource(message));
+ return Validate(*resource);
+ }
+
  std::vector<ValidationResult> results;
  Validate(message.GetDescriptor()->name(), message.GetDescriptor()->name(),
  internal::WorkspaceMessage(&message), &results);

cc/google/fhir/fhir_path/fhir_path_validation.h

@@ -127,12 +127,13 @@ class ValidationResults {
 // This class is thread safe.
 class FhirPathValidator {
  public:
- FhirPathValidator(const PrimitiveHandler* primitive_handler)
+ explicit FhirPathValidator(const PrimitiveHandler* primitive_handler)
  : primitive_handler_(primitive_handler) {}
  virtual ~FhirPathValidator();
 
+ // Validates the fhir_path_constraint annotations on the given message.
  ABSL_MUST_USE_RESULT
- ValidationResults Validate(const ::google::protobuf::Message& message);
+ absl::StatusOr<ValidationResults> Validate(const ::google::protobuf::Message& message);
 
  private:
  // A cache of constraints for a given message definition
@@ -170,11 +171,6 @@ class FhirPathValidator {
  constraints_cache_;
 };
 
-// Validates the fhir_path_constraint annotations on the given message.
-ABSL_MUST_USE_RESULT
-ValidationResults ValidateMessage(const PrimitiveHandler* primitive_handler,
- const ::google::protobuf::Message& message);
-
 } // namespace fhir_path
 } // namespace fhir
 } // namespace google

cc/google/fhir/fhir_path/fhir_path_validation_test.cc

@@ -25,6 +25,7 @@
 #include "absl/status/status.h"
 #include "google/fhir/fhir_path/r4_fhir_path_validation.h"
 #include "google/fhir/fhir_path/stu3_fhir_path_validation.h"
+#include "google/fhir/status/status.h"
 #include "google/fhir/status/statusor.h"
 #include "google/fhir/testutil/fhir_test_env.h"
 #include "proto/google/fhir/proto/r4/core/datatypes.pb.h"
@@ -77,7 +78,7 @@ template <typename T>
 class FhirPathValidationTest : public ::testing::Test {
  public:
  static ValidationResults Validate(const ::google::protobuf::Message& message) {
- return typename T::FhirPathValidator().Validate(message);
+ return *typename T::FhirPathValidator().Validate(message);
  }
 };
 
@@ -134,6 +135,14 @@ T ValidUsCorePatient() {
  )proto");
 }
 
+TYPED_TEST(FhirPathValidationTest, EmptyContainedResource) {
+ auto resource = ParseFromString<typename TypeParam::ContainedResource>("");
+
+ FHIR_ASSERT_STATUS(
+ typename TypeParam::FhirPathValidator().Validate(resource).status(),
+ "ContainedResource is empty.");
+}
+
 TYPED_TEST(FhirPathValidationTest, ConstraintViolation) {
  auto organization = ParseFromString<typename TypeParam::Organization>(
  R"proto(
@@ -268,8 +277,10 @@ TEST(FhirPathValidationTest, MessageLevelConstraintViolated) {
  end: { value_us: 1556750000000000 timezone: "America/Los_Angeles" }
  )proto");
 
- EXPECT_FALSE(
- r4::FhirPathValidator().Validate(end_before_start_period).IsValid());
+ FHIR_ASSERT_OK_AND_ASSIGN(
+ ValidationResults results,
+ r4::FhirPathValidator().Validate(end_before_start_period));
+ EXPECT_FALSE(results.IsValid());
 }
 
 TYPED_TEST(FhirPathValidationTest, NestedMessageLevelConstraint) {
@@ -295,15 +306,20 @@ TEST(FhirPathValidationTest, NestedMessageLevelConstraintViolated) {
  }
  )proto");
 
- EXPECT_FALSE(
- r4::FhirPathValidator().Validate(end_before_start_encounter).IsValid());
+ FHIR_ASSERT_OK_AND_ASSIGN(
+ ValidationResults results,
+ r4::FhirPathValidator().Validate(end_before_start_encounter));
+ EXPECT_FALSE(results.IsValid());
 }
 
 // TODO: Templatize tests to work with both STU3 and R4
 TEST(FhirPathValidationTest, ProfiledEmptyExtension) {
  r4::uscore::USCorePatientProfile patient =
  ValidUsCorePatient<r4::uscore::USCorePatientProfile>();
- EXPECT_TRUE(r4::FhirPathValidator().Validate(patient).IsValid());
+
+ FHIR_ASSERT_OK_AND_ASSIGN(ValidationResults results,
+ r4::FhirPathValidator().Validate(patient));
+ EXPECT_TRUE(results.IsValid());
 }
 
 TEST(FhirPathValidationTest, ProfiledWithExtensionsR4) {
@@ -318,7 +334,9 @@ TEST(FhirPathValidationTest, ProfiledWithExtensionsR4) {
 
  patient.mutable_birthsex()->set_value(r4::uscore::BirthSexValueSet::M);
 
- EXPECT_TRUE(r4::FhirPathValidator().Validate(patient).IsValid());
+ FHIR_ASSERT_OK_AND_ASSIGN(ValidationResults results,
+ r4::FhirPathValidator().Validate(patient));
+ EXPECT_TRUE(results.IsValid());
 }
 
 TEST(FhirPathValidationTest, ProfiledWithExtensionsSTU3) {
@@ -332,7 +350,9 @@ TEST(FhirPathValidationTest, ProfiledWithExtensionsSTU3) {
 
  patient.mutable_birthsex()->set_value(stu3::uscore::UsCoreBirthSexCode::MALE);
 
- ASSERT_TRUE(stu3::FhirPathValidator().Validate(patient).IsValid());
+ FHIR_ASSERT_OK_AND_ASSIGN(ValidationResults results,
+ stu3::FhirPathValidator().Validate(patient));
+ EXPECT_TRUE(results.IsValid());
 }
 
 } // namespace

cc/google/fhir/resource_validation.cc

@@ -151,8 +151,8 @@ ::absl::Status Validate(const ::google::protobuf::Message& resource,
 
  // TODO: Consider using the ErrorReporter in the FHIRPath library
  // as well rather than translating ValidationResults here.
- const fhir_path::ValidationResults results =
- message_validator->Validate(resource);
+ FHIR_ASSIGN_OR_RETURN(const fhir_path::ValidationResults results,
+ message_validator->Validate(resource));
  for (const fhir_path::ValidationResult& result : results.Results()) {
  if (!result.EvaluationResult().ok()) {
  // Report failures to evaluate a FHIRPath expression against the incoming
@@ -195,7 +195,9 @@ absl::Status ValidateResourceWithFhirPath(
  FHIR_RETURN_IF_ERROR(
  ValidateFhirConstraints(resource, resource.GetDescriptor()->name(),
  primitive_handler, FailFastErrorReporter::Get()));
- return message_validator->Validate(resource).LegacyValidationResult();
+ FHIR_ASSIGN_OR_RETURN(const fhir_path::ValidationResults results,
+ message_validator->Validate(resource));
+ return results.LegacyValidationResult();
 }
 
 absl::Status ValidateResource(const Message& resource,