FirelyTeam / Vonk.Facade.Starter

Example project for building a FHIR Facade with Vonk components
12 stars 9 forks source link

Exceptions when building Predicates are ignored #62

Closed bcbeatty closed 2 years ago

bcbeatty commented 3 years ago

I'm building a Predicate using the QueryFactory like in the walkthrough. I'm passing a parameter like this: intentionally swapping code and system

AllergyIntolerance?clinical-status=active|http://terminology.hl7.org/CodeSystem/allergyintolerance-clinical

which is an invalid query since the url should be

AllergyIntolerance?clinical-status=http://terminology.hl7.org/CodeSystem/allergyintolerance-clinical|active

To Reproduce

I have the following methods 
  public override AllergyIntoleranceQuery AddValueFilter(string parameterName, TokenValue value)
        {
            if (parameterName == "clinical-status")
            {
                return FilterByClinicalStatus(value);
            }

            if (parameterName == "_id")
            {
                return FilterById(value);
            }

            return base.AddValueFilter(parameterName, value);
        }

private AllergyIntoleranceQuery FilterByClinicalStatus(TokenValue value)
        {
            if (value.Code == null)
            {
                throw new ArgumentException("Allergy Clinical Status must be a valid value.", "code");
            }

            if (value.System == null || !value.System.Equals("http://terminology.hl7.org/CodeSystem/allergyintolerance-clinical"))
            {
                throw new ArgumentException("Not a valid System for the Allergy Intolerance Clinical Status.", "system");
            }

            return value.Code
                switch
            {
                "active" => PredicateQuery(allergy => !allergy.Discontinued),
                "inactive" => PredicateQuery(allergy => allergy.Discontinued),
                _ => throw new ArgumentException("Not a valid code. Please provide a correct code from system:http://terminology.hl7.org/CodeSystem/allergyintolerance-clinical", "code"),
            };
        }

Expected behavior When the query is executed I expect an ArgumentException to be thrown, except the exception is swallowed and not even logged

Version used:

Additional context Add any other context about the problem here.

ewoutkramer commented 3 years ago

I am pretty sure (seeing your version number) that this is for Firely Server.

alexzautke commented 3 years ago

Transferring issue to https://github.com/FirelyTeam/Vonk.Facade.Starter

frank-olthuijsen commented 3 years ago

Hi @bcbeatty, can you check if your code actually gets hit? If not, then please check if you've added "AllergyIntolerance.clinical-status" to "SupportedModel" > "RestrictToSearchParameters" in your Firely Server appsettings? Thank you.

bcbeatty commented 3 years ago

Yes, this is our SupportedModel section "SupportedModel": { "RestrictToResources": [ "Patient", "AllergyIntolerance" ], "RestrictToSearchParameters": [ "Resource._id", "Patient.given", "Patient.family", "Patient.gender", "StructureDefinition.url", "AllergyIntolerance.patient", "AllergyIntolerance.clinical-status" ] },

I've run the code in debug mode and stepped through. The throw new ArgumentException("Not a valid System..." does get thrown. but the client call

 var query = _queryContext.CreateQuery(queryFactory, arguments, options);

does not have any reference to an exception being thrown or caught

frank-olthuijsen commented 3 years ago

@bcbeatty I was able to reproduce this. The exception indeed gets swallowed and doesn't end up in the logs either. We will investigate.

bcbeatty commented 3 years ago

Further investigation on our side ignores the predicate and returns all records unfiltered.

EugOSullivan commented 2 years ago

Hi. Was this issue resolved or a workaround example in place? I'm seeing the same behaviour in my façade implementation and same as bcbeatty ignores predicate and returns all. Thanks

alexzautke commented 2 years ago

Hi @EugOSullivan,

the current behaviour is the intended behaviour although I agree that it can be unexpected. I've added a task to our next sprint in the Firely Server Team to extend the documentation on this topic.

The server is free to ignore search parameters, therefore exceptions are being translated to an error on the argument representing the search parameter. Any exception being thrown can be detected again by checking the argument collection for the search repository.

Let me know if you need any additional help.

alexzautke commented 2 years ago

See https://github.com/FirelyTeam/firely-docs-firely-server/pull/140 for more details