google / fhir

FHIR Protocol Buffers
Apache License 2.0
830 stars 189 forks source link

go: profiling + validation support in go #22

Open tmc opened 4 years ago

tmc commented 4 years ago

We're maintaining some custom resource validation code that we'd love to deprecate in favor of profiling support in this codebase.

Can you give an estimate on when we might see profiling+validation support for go in this repo?

nikklassen commented 4 years ago

What kind of profiling/validation are you looking for as part of these libraries? Some is already baked in, and we'd like to separate it out explicitly. Full validation of a StructureDefinition is not in the scope of this library right now.

nickgeorge commented 4 years ago

Hi Travis, To expand a bit on what Nik said, there's a couple of things to keep in mind (sorry if you know all of this!)

Profiles are generated using a standalone process - these are essentially meta descriptions that describe your data, so they're typically not modified in-process. The code itself is written in Java, but that doesn't matter a whole lot, you can use the generated profile protos in any language you like.

Similarly, the validation libraries themselves don't particularly care if something is a "profile" or not - they use the proto annotations in the same ways to validate.

In current state, Go kind of lumps validation and parsing together in a way that makes it hard to use separately - it's definitely on the roadmap to separate those two, but it's a bit hard to say exactly when that would happen - I'd guess next quarter but maybe Nik can give a better estimate.

So the final piece to all this puzzle is the profiler itself, which I'm guessing is the core of your question - this is the ability to say "Take data in core resources, and convert it to this profile." Unfortunately, that's gonna take porting a decent amount of code from C++ and isn't currently a huge priority for the Go folks so I'm not sure I can promise it in the immediate future.

tmc commented 4 years ago

Thanks for the responses. To clarify, we're generating protos using the java code in the repository but and the things I'm after in terms of go support are the validation of fields (e.g. REQUIRED_BY_FHIR) as well as code for conversion between a profiled and non-profiled representation of resources.

It sounds like some of the validation code is in the pipes but we may be on the line for maintaining code to perform conversions to/from profiled representations. Is that accurate?

nickgeorge commented 4 years ago

Yeah that sounds right to me. Maybe you could do some kind of swig style wrapper around https://github.com/google/fhir/blob/master/cc/google/fhir/r4/profiles.h ?

nikklassen commented 4 years ago

The Go code does support the validation fields like "required by FHIR", but only if it's the Go code parsing from JSON to proto. Like Nick said, validation of a Go proto struct that has already been parsed is probably coming in the next quarter. The profiled to unprofiled conversion hasn't been a feature we considered for Go yet, so if you can use the C++ code that would be your best bet.

tmc commented 4 years ago

AFAICT that code doesn't work today with new Go types generated from profiles (e.g. won't be in the list of possible oneofs for ContainedResource). Is that true?

nickgeorge commented 4 years ago

ContainedResource behavior is governed by the PackageInfo#contained_resource oneof: https://github.com/google/fhir/blob/master/proto/profile_config.proto#L26 If you set local_contained_resource to true, it will generate a new ContainedResource to use throughout the profile. Make sure to add a Bundle profile to your profile set so you have a Bundle that has Entry with the right ContainedResource.

Exciting to hear about external usages of profiles!

nickgeorge commented 4 years ago

Although thinking a bit more, you might have limited library support after you convert... unlike the C++ library, which is reflective + generic, some APIs in the Go library expect core profiles. But if you wrap the "profile + validate" C++ APIs, you should be able to get solid validation reports back from your protos, and then manipulate them in Go.

tmc commented 4 years ago

ContainedResource behavior is governed by the PackageInfo#contained_resource oneof: https://github.com/google/fhir/blob/master/proto/profile_config.proto#L26 If you set local_contained_resource to true, it will generate a new ContainedResource to use throughout the profile. Make sure to add a Bundle profile to your profile set so you have a Bundle that has Entry with the right ContainedResource.

Exciting to hear about external usages of profiles!

Oh we overlooked that! Thanks for pointing that out. I'm guessing currently the jsonformat apis aren't going to be compatible with an alternate ContainedResource type (as I think you're hinting at).

nickgeorge commented 4 years ago

yeah, that's right, that's why I'm recommending doing the parsing and validating in C++ for profiles.

nikklassen commented 3 years ago

As of 0.6.3 we have launched the fhirvalidate package that can enforce checks like required fields in Go without going through the parser.