FirelyTeam / firely-net-sdk

The official Firely .NET SDK for HL7 FHIR
Other
824 stars 342 forks source link

SnapshotSource does not reuse self-generated snapshot #2802

Closed cknaap closed 3 weeks ago

cknaap commented 3 months ago

Describe the bug SnapshotSource should return the same snapshot upon multiple retrievals. Instead, it regenerates the snapshot every time.

To Reproduce Steps to reproduce the behavior:

  1. Create a SnapshotSource, with regenerate: true, based on a CachedResolver
  2. Get a StructureDefinition by canonical, e.g. from Element
  3. Save a reference to the resulting StructureDefinition.Snapshot
  4. Repeat step 2
  5. Save a reference to the second resulting StructureDefinition.Snapshot
  6. Observe that both references are not the same.

Expected behavior When the SnapshotSource has generated (by the Generator) a snapshot, it is marked as IsCreatedBySnapshotGenerator(), and it should not be re-generated again upon subsequent retrievals.

Screenshots n/a

Version used:

Additional context

ewoutkramer commented 3 months ago

This is not a bug, but a feature. Set regenerate to false to get the desired behaviour. I must be missing something ;-)

ewoutkramer commented 3 months ago

Offline discussion added some new insights:

If we'd apply #2803 we will certainly get regressions, since this PR will cause true to result in the old behaviour for false, and false (still the default) will cause 3rd-party snapshots to be used instead of being regenerated.

A first conclusion might be that the current regenerate option is not particularly useful (we'd be better of without it), is not documented clearly and does not allow for 3rd-party snapshots to be used. We should obsolete it and introduce a more explicit setting. Basically there are only two cases: