Add ability for for clients to provide their own instance parsing #756
Conversation
| fileInstanceParsers = Stream.concat( | ||
| Stream.of(fileInstanceParser), | ||
| fileInstanceParsers.stream() | ||
| ).collect(Collectors.toList()); |
There was a problem hiding this comment.
This got a little bit of an eyebrow raise from me. 😅 Is it taking a streaming approach too far? Do you want to avoid fileInstanceParsers.add(0, fileInstanceParser) to avoid direct mutation? Is it substantively different here? Is it to be consistent with other stream-based list processing?
I don't feel strongly but wanted to share my eyebrow raise.
There was a problem hiding this comment.
It's "completely forgetting about add(0, fileInstanceParser) and trying to just append two lists". Remember we don't have to deal with any of this insanity in Collect any longer!
There was a problem hiding this comment.
Derp. Can't do that because it's an immutable list. I already merged 🤦♂️. Sigh.
|
Looks good! Feel free to merge after taking or leaving my one comment! |
seadowg
force-pushed
the
instance-interceptor
branch
from
c4acf68 to
ef7afa4
Compare
Work towards getodk/collect#6029
This adds a new
FilterInstanceParserinterface that is now used byGeoJsonExternalInstanceandCsvExternalInstanceand allows clients to add their own implementations.What has been done to verify that this works as intended?
Verified with a spike in Collect.
Why is this the best possible solution? Were any other approaches considered?
I tired using an "interceptor"/"preprocessor" approach, but then realised that the way we parse CSV and GeoJSON files is already a "chain" of parsers so formalising that made the most sense.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Shouldn't change behaviour - it adds the ability for clients to change behaviour from their side.