test(db): expand /api/db/permissions coverage + document accepted mode formats#47
Open
joewiz wants to merge 2 commits into
Open
test(db): expand /api/db/permissions coverage + document accepted mode formats#47joewiz wants to merge 2 commits into
joewiz wants to merge 2 commits into
Conversation
The endpoint accepts owner/group/mode (all optional), but the existing
test only exercised mode-on-a-resource with one rwxrwxrwx-style value.
Real coverage was shallow. Adding the cases that actually matter for
clients (eXide DB Manager, Oxygen plugin) before they round-trip
against the endpoint:
- owner change on a resource (admin → guest → admin)
- group change on a resource (dba → guest → dba)
- combined {owner, group, mode} in a single request — verify all three
applied
- mode change on a collection (sm:chmod handles collections same as
resources, but worth pinning since the eXide UI surfaces this)
- octal mode '0644' → rejected. Documenting the contract:
sm:chmod only accepts the symbolic rwxrwxrwx form (and relative
u+x/g-w/...), not octal strings. Clients that want to send a numeric
mode must convert it first.
- missing 'path' → HTTP 400 (declared in spec but previously untested)
- nonexistent path → HTTP 5xx with a sensible error description
Each round-trip-style test now verifies via GET /api/db/properties
that the requested field actually changed, then restores the original
value so later tests in the same suite aren't affected.
Setup creates a dedicated permResource + permSubCollection in the
test collection so these tests don't perturb the earlier /test.xml
mode-change test (and vice versa); cleanup is handled by the existing
testCollection teardown.
All 50 db.cy.js tests pass (up from 43).
…ccepts Spell out in the OpenAPI spec which mode-string formats sm:chmod's string overload accepts, and call out that octal strings (0644 / 644) are NOT accepted — this is the same contract the new octal-rejection test pins. Clients now have the formats documented up front instead of having to file an issue when a typed numeric mode fails. If a client comes back asking for octal support, the test flips to assert success and the handler gains a small parse-and-coerce step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[This PR was co-authored with Claude Code. -Joe]
POST /api/db/permissionsacceptsowner/group/mode(all optional), but the existing test exercised only one path: mode change on a resource, singlerwxrwxrwx-style value. Coverage was shallow. Filling in the gaps so we have real confidence in the endpoint before eXide's DB Manager round-trips against it.New tests (7)
ownerchange on a resource/properties, restore to admingroupchange on a resource{owner, group, mode}combined in one requestmodechange on a collection (not just a resource)sm:chmodtreats collections the same way, but the eXide UI surfaces this separately, so worth pinning'0644'→ rejectedsm:chmodonly accepts the symbolicrwxrwxrwxform (and relativeu+x/g-w/ …), NOT octal strings. Clients that want to send a numeric mode must convert it first.path→ HTTP 400Each round-trip test verifies via
GET /api/db/propertiesthat the requested field changed, then restores the original value so later tests in the suite aren't affected.Setup creates a dedicated
permResourceandpermSubCollectioninside the existingtestCollectionso these tests don't perturb (or get perturbed by) the earlier/test.xmlmode-change test; cleanup is handled by the existingtestCollectionteardown.Result
All 50
db.cy.jstests pass — up from 43. No production-code change in this PR; this is pure test infill against existing endpoint behavior, surfacing the contract for clients to rely on.Discovery worth flagging: octal mode strings are silently rejected
While building the tests I found that
POST /api/db/permissionswith"mode": "0644"(or"644") fails withUnrecognised mode syntax. Tracked it toAbstractUnixStylePermission.setMode(String)in eXist core, which accepts exactly three symbolic formats — simplerwxr-xr-x, Unix relativeu+x,g-w, eXist verboseuser=+read,…— and not octal strings. eXist does support octal via the integer overload ofsm:chmod, but existdb-openapi's handler passes$body?mode(a JSON string) to the string overload, which doesn't try parsing as octal.Checked the impact on real clients:
directory.js:298-313— e.g."u+r,u-w,u+x,g+r,…"). Already works.Octal-as-string only burdens callers who think in
chmod 644terms (scripts, CLI tools, docs ported from Unix tutorials). No current first-party client hits it.What this PR does about it: pin the current behaviour as a test, document the three accepted formats and the octal omission in
api.jsonso the contract is on the spec page. If a client comes back wanting octal, the implementation is small — detect^0?[0-7]{3,4}$in the handler, convert toxs:integer, dispatch to the integer overload ofsm:chmod— and the test flips from "rejected" to "coerced and applied". Open an issue at that point.Why now
Per the eXide-side conversation: before adding round-trip permission-change tests to eXide's DB Manager cypress suite, the underlying existdb-openapi endpoint should have proper coverage. With this PR, the eXide tests can confidently treat the endpoint as a stable contract.