Skip to content

Persist a PATCH to the partnerships file for the XML store - #611

Merged
uhurusurfa merged 1 commit into
OpenAS2:masterfrom
JakeHuneau:fix/patch-persist-xml-store
Sep 16, 2026
Merged

uhurusurfa merged 1 commit into
OpenAS2:masterfrom
JakeHuneau:fix/patch-persist-xml-store

Conversation

@JakeHuneau

Copy link
Copy Markdown
Contributor

A PATCH against the XML partnership store reports success but never writes the file, so the change is lost.

Cause

ApiResource.processRequest stores the partnerships XML after a successful change, but the trigger only lists two of the three actions:

if (CommandResult.TYPE_OK.equals(output.getType()) && resource.startsWith("partner")
        && ("add".equals(action) || "delete".equals(action))) {

The PATCH endpoint added in 4.12.0 runs the update command, which is not in that list. For the XML store an update therefore only changes the in-memory document. The caller is told the update succeeded, then the change is lost on restart and silently reverted by the next partnerships refresh.

The database store is unaffected: it persists each change in its own transaction.

Fix

Add update to the trigger. This is correct for both stores, because StorePartnershipsCommand already recognises the database store and returns "The database partnership store persists changes immediately so there is nothing to do", so only the XML store changes behaviour. A failed update still does not store, since the trigger requires an OK result.

Testing

176 tests pass, 3 new in PatchApiTest, which exercises the XML store over real HTTP. Reverting the one line fails the first two.

  • patchIsPersistedToThePartnershipsFileNotJustMemory patches a partner then reads partnerships.xml back off disk and requires the new value to be there
  • patchReportsThatItStoredThePartnerships requires the response to report the store, as it does for add and delete
  • aFailedPatchDoesNotRewriteThePartnershipsFile requires a rejected update to leave the file byte identical

One consequence worth noting: storePartnership writes a numbered backup before each save, so a script that patches many partners in sequence now leaves one backup per call. That was already true for add and delete.

@uhurusurfa uhurusurfa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

The REST layer stores the partnerships XML after a successful add or delete but
the PATCH endpoint added in the previous release runs the update command, which
was not in that list. For the XML store an update therefore only changed the in
memory document: it was lost on restart and silently reverted by the next
partnerships refresh, with the caller having been told the update succeeded.

Storing after an update is correct for both stores. The store command already
recognises the database store and reports that it has nothing to do, because
that store persists each change in its own transaction, so this only changes
behaviour for the XML store.

A failed update does not store, since the trigger requires an OK result.
@JakeHuneau
JakeHuneau force-pushed the fix/patch-persist-xml-store branch from 527db6e to 0e1df02 Compare September 16, 2026 10:26
@uhurusurfa
uhurusurfa merged commit be63798 into OpenAS2:master Sep 16, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants