VNG-Realisatie / gemma-zaken

Samen ontwikkelen van API's voor Zaakgericht werken
https://vng-realisatie.github.io/gemma-zaken/
Other
41 stars 27 forks source link

Hoe om te gaan met versiebeheer bij meerdere updates en gelijkblijvend lockId? #2454

Open basretera opened 2 months ago

basretera commented 2 months ago

Hoe moeten we als DRC omgaan met versienummering bij meerdere updates (PUT, PATCH) waarbij het lockId gelijk blijft?

Scenario:

  1. Applicatie lockt het EIO om een update uit te voeren.
  2. Applicatie update het EIO nu meerdere malen achter elkaar. Dus voert meerdere PUTs of PATCHes uit
  3. Applicatie unlocked het EIO.

Wat is er nu met het versienummer van het document gebeurd? Zeker indien in de put of patch ook de inhoud/bestandsdelen zijn gewijzigd? Krijgt iedere update een nieuw versienummer of krijgt het document pas een nieuw versienummer bij de unlock? En zijn daarmee alle tussenliggende 'versies' zogenaamde draftversies/conceptversies?

Een bijkomende 'probleem' hierbij. Stel de applicatie update de inhoud middels bestandsdelen maar unlocked het EIO nog niet en update vervolgens direct de inhoud weer met bestandsdelen? Welke bestandsdelen zijn dan nog de goede? Hoe weet je dat? Moet een PUT of PATCH de vorige openstaande bestandsdelen verwijderen etc. etc.?

HenriKorver commented 1 month ago

Krijgt iedere update een nieuw versienummer of krijgt het document pas een nieuw versienummer bij de unlock?

Net even gespeeld met de referentie-implementatie en daar krijg je pas een nieuw versienummer na de unlock. Dus tijdens de lock wordt het versienummer niet opgehoogd ook niet als je meerdere updates doet (bijv. met PATCH). Er zit wel een bug in merkte ik: na de unlock wordt de versie niet meteen opgehoogd, nee dat gebeurt pas bij de volgende PATCH-operatie, vreemd dus.

@sergei-maertens, @joeribekker: Hoe werkt dit in OpenZaak? En hebben jullie geen last van de genoemde bug?

sergei-maertens commented 1 month ago

ik herinner me dat het gedrag hiervan gewijzigd is tussen Documenten API 1.0 en Documenten API 1.1, wegens de bestandsdelen feature inderdaad. Ik weet niet meer uit het hoofd hoe dit zit in Open Zaak, maar dit zal wel de referentie-implementatie volgen, of is in ieder geval de bedoeling: versienummer bump na de unlock (want dan "commit" je de wijzigingen, soort van)

@annashamray can you provide some input?

annashamray commented 1 month ago

If I understood the question correctly, in Open Zaak the new version of the document is created with each PUT/PATCH request. So, if you lock the document, update it 3 times and then unlock, 3 new versions would be created

basretera commented 1 month ago

Open Zaak gaat hier dus anders mee om dan de referentie-implementatie als ik het zo lees. Hoe moeten we hier nu mee omgaan?

sergei-maertens commented 1 month ago

Still doing some observations, so please hold your horses :-)

Anna did some more checks and the described behaviour is indeed what happens.

I think it would be a good idea to formalize what the expected behaviour is here. Both flavours don't seem inherently wrong to me - however I do think it would be weird if you lock -> unlock a document without making any changes, that this would result in a version number changes since nothing has been changed. So the semantic meaning of the version number incrementing on PUT/PATCH makes sense to me.

I think in OZ it was decided to just implement it like this because it's the most simple way, instead of having to look up if a particular version already exists and compare it with the non-locked version etc...

sergei-maertens commented 1 month ago

Een bijkomende 'probleem' hierbij. Stel de applicatie update de inhoud middels bestandsdelen maar unlocked het EIO nog niet en update vervolgens direct de inhoud weer met bestandsdelen? Welke bestandsdelen zijn dan nog de goede? Hoe weet je dat? Moet een PUT of PATCH de vorige openstaande bestandsdelen verwijderen etc. etc.?

IMO as long as the object has not been unlocked, it's a draft and you can do whatever you want. To get the bestandsdelen URLs/parts to upload to, you need to pass some file size first so that the server can determine how many/which parts need to be uploaded. If you need to overwrite a part for whatever reason, I think you should just be able to do that. each bestandsdeel call is a PUT and is supposed to overwrite whatever was there before, so as long as the document is still locked, you discard the old one and store the new one, only when the file is being unlocked do you stitch together all the parts and commit that to the storage. At least, that's how OZ implements it.

basretera commented 1 month ago

Dank @sergei-maertens voor het uitzoeken.

I think it would be a good idea to formalize what the expected behaviour is here.

Willen/kunnen jullie een besluit nemen en een uitspraak doen zodat we weten waar we aan toe zijn? Nog beter: opnemen in de documentatie en/of API specificaties.

michielverhoef commented 1 month ago

Dit is vergelijkbaar met lokaal werken en een versiebeheersysteem zoals svn of git: Een bestand wat lokaal gewijzigd wordt wordt niet direct gecommit. Pas bij een commit wordt een nieuwe versie in het versiebeheersysteem gemaakt, niet bij het lokaal opslaan van je gewijzigde bestand.

Een PUT/PATCH van een informatieobject is een commit en leidt tot een nieuwe versie. Mij staat bij dat na een PUT of PATCH het informatieobject vroeger ook direct ge-unlocked werd. De unlock operatie was meer bedoeld als undo checkout. Wanneer dat principe gevolgd wordt zou het issue wat hier genoemd wordt ook niet voor kunnen komen, je kunt met een lockid immers maar 1 keer een informatieobject wegschrijven. Mogelijk is dat gewijzigd met Documenten API 1.1 en de multiparts.

In theorie kun je prima een informatieobject opvragen en lokaal bewerken, zonder een lock. Alleen terugschrijven kan niet.

sergei-maertens commented 1 month ago

je kunt met een lockid immers maar 1 keer een informatieobject wegschrijven

dit was nooit zo - als jij de eigenaar/houder bent van het lock, dan kan je zo vaak muteren als je wil, het betekent alleen dat andere niet kunnen muteren want zij kennen het lock-id niet :) dus het is prima mogelijk (en wenselijk zelfs?) dat je dit kan doen:

Deze mutaties zijn ook wijzigingen in metadata, maar ook bestandsinhoud. Voor bestandsdelenuploads waarbij meer dan 1 bestandsdeel toegevoegd moet worden heb je automatisch meerdere schrijfoperaties met hetzelfde lock ID, per definitie.

HenriKorver commented 3 weeks ago

Willen/kunnen jullie een besluit nemen en een uitspraak doen zodat we weten waar we aan toe zijn?

Mijn voorstel is om de referentie-implementatie te volgen. Dit houdt in dat de versie alleen wordt opgehoogd na een "unlock" indien er wijzigingen hebben plaatsgevonden. Tijdens de lock kunnen meerdere mutaties plaatsvinden maar die zullen allemaal samen resulteren in één ophoging van de versie met +1. In het speciale geval als er geen wijzigingen hebben plaatsgevonden tijdens de lock zal de versie niet worden opgehoogd na de "unlock".

joeribekker commented 3 weeks ago

Lijkt erop dat het in de RI al gebeurt bij de Lock zelfs :)

https://github.com/VNG-Realisatie/documenten-api/blob/6b763bd649f9db5d817f564ddd2ebccd741e099b/src/drc/api/serializers/enkelvoudig_informatieobject.py#L390-L420

Ik zie in Open Zaak dat we het gedrag gewijzigd hebben n.a.v. de koppeling via CMIS. Dit omdat elke keer als we iets wegschrijven naar CMIS er een versie ophoging is. Kan niet exact achterhalen waarom dit dan zonder CMIS ook zo gedaan is maar waarschijnlijk om de code beheersbaarder te houden en het gedrag consistent te houden.

Ik denk dat het semantisch het meest correct is om een versie op te hogen na de unlock mits er wijzigingen zijn geweest.