FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
816 stars 340 forks source link

fix: #2802 - let SnapshotSource reuse self-generated snapshot #2803

Open cknaap opened 3 weeks ago

cknaap commented 3 weeks ago

Description

If SnapshotSource detects a snapshot generated by our own SnapshotGenerator, it does not re-generate it.

Related issues

Fixes #2802.

Testing

See 2 new unit tests in the PR.

FirelyTeam Checklist

ewoutkramer commented 3 weeks ago

I do not understand why #2802 is considered a bug. When you set the setting "ForceRegenerateSnapshots" to true, it will re-generate the snapshots, no matter what. This is the documented behaviour of this option:

Force expansion of all external profiles, disregarding any existing snapshot components. If enabled, the snapshot generator will re-generate the snapshot components of all the core resource and datatype profiles as well as of all other referenced external profiles. Re-generated snapshots are annotated to prevent duplicate re-generation (assuming the provided resource resolver uses caching). If disabled (default), then the snapshot generator relies on existing snapshot components, if they exist.

This PR does not look like a fix, but a behavioural change. The "ForceRegenerateSnapshot" used to always regenerate the snapshot, now it will only renegerate it if it was not created by us. That's a breaking change that we should not make in a non-major version.

Why don't you just set ForceRegenerateSnapshot to false instead? This will create a new snapshot if there is none, or if the snapshot is not ours.

ewoutkramer commented 3 weeks ago

Design discussion will be continued on issue #2802.