fix(adms): align API surface with live ADMS wire format by ShiwaliKusum · Pull Request #192 · SAP/cloud-sdk-python

ShiwaliKusum · 2026-07-01T05:31:12Z

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Aligns the ADMS Python SDK API surface with the live ADM service wire format. Several entity paths, key structures, and method signatures were incorrect — they compiled but failed at runtime (HTTP 400/404) against the real service.

Configuration API fixes:

DocumentTypeBusinessObjectTypeMap uses a composite key (DocumentTypeID + BusinessObjectNodeTypeUniqueID), not a single UUID. get_type_mapping, delete_type_mapping, and mark_default updated accordingly.
FileExtensionPolicy entity set renamed from FileExtensionPolicy to DocumentTypeFileExtensionPolicy (matches $metadata). Model fields corrected to DocumentTypeID + FileExtension composite key; get_file_extension_policy(uuid) removed as no such UUID key exists in ADM.
CreateDocumentTypeBoTypeMapInput wire field names corrected from DocumentTypeId/BusinessObjectNodeTypeUniqueId to DocumentTypeID/BusinessObjectNodeTypeUniqueID.
MimeTypePolicy removed from public __init__ exports (no longer referenced by any public model).

Document API fixes:

update() (sync + async): removed the implicit follow-up GET — the UpdateDocument action returns the partial response directly from ADM; callers who need the full document should call get() explicitly.
get_download_url() (sync + async): doc_content_version_id is now optional (None → DownloadDocument() for latest version), matching the OData Core.OptionalParameter annotation.

Job API fixes:

StartJob and JobStatus called without namespace prefix (unbound action/function imports do not use namespace prefix per OData spec).

Related Issue

Closes #https://jira.tools.sap/browse/RBSADMS-47

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Code refactoring
Dependency update

How to Test

Configure credentials: set -a && source .env.adms && set +a
Run unit tests: uv run pytest tests/adms/unit/ -v
Run integration tests against a live ADM instance: uv run pytest tests/adms/integration/ -v
Verify cfl, cmg, cmd, cgf, jz, js commands work against the live tenant using scripts/adms_cli.py

Checklist

I have read the Contributing Guidelines
I have verified that my changes solve the issue
I have added/updated automated tests to cover my changes
All tests pass locally
I have verified that my code follows the Code Guidelines
I have updated documentation (if applicable)
I have added type hints for all public APIs
My code does not contain sensitive information (credentials, tokens, etc.)
I have followed Conventional Commits for commit messages

Breaking Changes

get_type_mapping and delete_type_mapping now require two arguments (document_type_id, business_object_node_type_unique_id) instead of one. However, the previous single-argument version accepted a DocumentTypeBOTypeMapID that the ADM service never supported — every call returned HTTP 400. Since the old signature was non-functional, this is treated as a bug fix rather than a breaking API change.

get_file_extension_policy(uuid) is removed. The ADM entity has no UUID primary key; policies are identified by (DocumentTypeID, FileExtension). The old method would have always returned HTTP 404.

update() no longer performs an implicit GET after updating a document. The return value is now the partial action response from ADM (only changed fields). Callers requiring the full document after an update should call documents.get() explicitly.

Additional Notes

All fixes were validated against a live ADM playground tenant. The scripts/adms_cli.py interactive CLI (kept locally, not included in this PR) was used for end-to-end verification of each endpoint.

cla-assistant · 2026-07-01T05:31:28Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ adwitiyasushant
❌ I766515

I766515 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

NicoleMGomes · 2026-07-01T12:42:19Z

PR description is not following our template. Only raise PRs to us once it's complete.

Adds the SDK Module Review skill to this repository: ## What this PR adds **20 deterministic checks** (no LLM calls in CI): - secrets, license-spdx, disclosure, hardcode, telemetry, docs, bdd, patterns, versioning + BREAKING family, commits, errors-logging, testing-depth, http-hygiene, concurrency, deps-supply, deletion-hygiene, constants, binding-shape, quality-gate-parity, pr-size **Rule tiers:** SHADOW | FLAG | BLOCK | BLOCK_LOCKED, configured in `.claude/config/rules.yaml` **BLOCK_LOCKED rules cannot be suppressed:** - SEC-* (all secrets), HC-03 (SAP-internal URL leak), DIS-06 (internal artifactory), LIC-01/02 (SPDX headers), BND-02 (BTP token URL concat), BND-05 (binding logs credentials), BREAKING-* (all breaking-change declarations) **REUSE.toml aggregate exemption** — this repo uses REUSE.toml at root, so per-file SPDX rules are baseline-exempted automatically. **Cross-language BDD parity** — module-aliases.yaml seeded with `dms ↔ documentmanagement` (Java sibling). **4 PR signals posted per run:** 1. Inline comments per finding 2. Aggregated summary comment 3. GitHub check-run (Checks tab) 4. Status label (`sdk-review: ✅ passed` / `❌ blocked` / `⚠️ flagged`) **Idempotent re-runs:** HTML markers let subsequent runs replace prior artifacts rather than duplicate them. ## Structure - `.claude/scripts/`: 20 check-*.sh + orchestrate.sh + aggregate.sh - `.claude/scripts/lib/`: 12 shared libs (2 Python AST + 10 bash) - `.claude/config/`: rules.yaml, module-aliases.yaml, baseline.json - `.github/workflows/sdk-module-review.yml`: auto-trigger on every PR - `.github/workflows/sdk-skill-isolation-check.yml`: validates sub-PRs - `docs/PR-REVIEW.md`: user-facing docs - `docs/BRANCH-PROTECTION-SETUP.md`: admin guide for required-check config - `tests/sdk-review/`: 57 bats tests + 6 fixtures (placeholder secrets, not real credentials) - `CONTRIBUTING.md`: amended — sdk-module-review is required before review ## Validation - 57/57 bats tests passing - shellcheck clean on all bash scripts - E2E on 5 real PRs (#196, #191, #192, #185 in this repo + Java jv#9): zero false positives observed - Self-test on this PR: 0 BLOCK, 0 FLAG (skill correctly skips its own files) - Flagship BND-02 finding verified on Java new-module PR (token URL concat) ## Rollout Recommended SHADOW → FLAG → BLOCK progression, 2 weeks each. See `docs/BRANCH-PROTECTION-SETUP.md`. ## Note on test fixtures `tests/sdk-review/fixtures/secrets-*.diff` contain intentional placeholder strings that match secret regexes (e.g. `AKIATESTFIXTUREXXXXX`, `eyJTESTFIXTURE...`) so the check-secrets rule can be unit-tested. These are NOT real credentials. Ref: https://jira.tools.sap/browse/AFSDK-3937

Mirrors cloud-sdk-java contrib-java/adms commit e5dd1da's wire-format audit. The Java audit identified 12 fixes against the canonical CDS sources + live ADMS tenant. Cross-verified Python state vs CDS: - 7 fixes were already correct in Python (UpdateDocumentInput fields, RestoreContentVersion envelope, DeleteContentVersion body shape, DownloadDocument GET function, StartJob payload, JobStatus function, LateHostBusinessObjectNodeID casing). - 5 namespace-prefix fixes were genuinely missing (lock, unlock, complete_multipart_upload, generate_upload_urls, mark_default) — Python used the short form which CAP accepts leniently but is not the canonical OData V4 wire format. For cross-SDK consistency with Java, ALL 12 bound/unbound action paths now use the fully-qualified com.sap.adm.{DocumentService,AdminService, ConfigurationService}.X form: DocumentService bound actions on DocumentRelation: UpdateDocument, CompleteMultipartUpload, RestoreDocumentContentVersion, DeleteDocumentContentVersion, LockDocumentAndRelation, UnlockDocumentAndRelation, GenerateDocumentUploadURLs, DownloadDocument (function) DocumentService unbound: StartJob, JobStatus (function) AdminService unbound: StartJob, JobStatus (function) ConfigurationService bound on DocumentTypeBusinessObjectTypeMap: markDefault Test changes: - Tightened 7 substring assertions in test_client.py to include the namespace prefix so the wire contract is captured in tests. Also includes prior work on this branch: - extra_headers support on sync + async HTTP verbs (for x-subaccount-id header on ApplicationTenant operations) - subaccount_id parameter on all 4 ApplicationTenant config methods No API surface changes, no model changes, no behavioural changes — purely wire-protocol alignment with Java commit e5dd1da.

Interactive CLI for testing all ADMS APIs against a real ADM instance. Used during this PR's development to verify each of the 12 wire-format fixes returns the expected response from the live tenant. WILL BE REMOVED BEFORE MERGE — kept in this PR only as a debugging aid so reviewers can reproduce the live-tenant verification if desired. Usage: set -a && source .env.adms && set +a .venv/bin/python scripts/adms_cli.py

- Remove MimeTypePolicy from __init__.py (renamed to FileExtensionPolicy in _models.py) - Rename FileExtensionPolicy entity set to DocumentTypeFileExtensionPolicy - Drop get_file_extension_policy by UUID; delete now uses composite key (DocumentTypeID + FileExtension) - Fix build_doctype_botype_map_key_path to use composite key (DocumentTypeID + BusinessObjectNodeTypeUniqueID) - Update get_type_mapping / delete_type_mapping / mark_default to accept composite key params - Make doc_content_version_id optional in get_download_url (omit parentheses content when None) - Drop post-update GET in update_document; return partial response from UpdateDocument action directly - Fix X-SubaccountId header casing - Align job API namespace constants with canonical server wire format - Expand adms_cli.py with rfu (full upload) command and additional fixes from live tenant testing

…lt type mapping The ADM DocumentTypeBusinessObjectTypeMap entity uses a composite key (DocumentTypeID + BusinessObjectNodeTypeUniqueID). The previous single-argument signature used a fabricated DocumentTypeBOTypeMapID that was never accepted by the service (HTTP 400), so this signature change is a bug-fix, not a breaking API change. Added note in docstrings to clarify.

betinacosta · 2026-09-15T18:30:29Z

Several lines in this file seems to exceed 88 characters substantially, which is probably why pre-commit is failing.

betinacosta · 2026-09-15T18:33:57Z

+    return {"X-SubaccountId": subaccount_id} if subaccount_id else None
+
+
 def _quote_guid(value: str) -> str:


It seems that this method is no longer used.

betinacosta · 2026-09-15T18:37:58Z

The new ApplicationTenant operations introduce a subaccount_id parameter that routes to a per-tenant context, which is precisely the multi-tenancy routing pattern. The user-guide.md claims multi-tenancy is "Not Supported", but this PR adds subaccount-scoped operations — this is at minimum inconsistent and should be clarified. Additionally, the async delete_application_tenant (_configuration_api.py:766-769) does not pass extra_headers=_subaccount_header(subaccount_id) — the subaccount_id parameter is accepted but silently dropped on the async path.

betinacosta · 2026-09-15T18:40:06Z

New methods create_file_extension_policy, delete_file_extension_policy, get/create/delete ApplicationTenant, and mark_default lack unit tests.

ShiwaliKusum requested a review from a team as a code owner July 1, 2026 05:31

NicoleMGomes reviewed Jul 1, 2026

View reviewed changes

Comment thread scripts/adms_cli.py Outdated

NicoleMGomes reviewed Jul 1, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/adms/_configuration_api.py Outdated

ShiwaliKusum requested a review from NicoleMGomes July 3, 2026 07:29

adwitiyasushant and others added 8 commits September 2, 2026 13:56

remove scripts/adms_cli.py from branch (keep local only)

10e4438

chore: trigger CI

d6599de

chore: bump version to 0.48.3

5397cae

chore: restore workflow file to avoid token scope issue

fa0929c

ShiwaliKusum force-pushed the feat/pr-184 branch from 86e7359 to fa0929c Compare September 2, 2026 08:27

thiagob requested a review from ricardosrib September 14, 2026 17:25

betinacosta reviewed Sep 15, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(adms): align API surface with live ADMS wire format - #192

fix(adms): align API surface with live ADMS wire format#192
ShiwaliKusum wants to merge 8 commits into
SAP:mainfrom
ShiwaliKusum:feat/pr-184

ShiwaliKusum commented Jul 1, 2026 •

edited

Loading

Uh oh!

cla-assistant Bot commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

NicoleMGomes commented Jul 1, 2026

Uh oh!

betinacosta Sep 15, 2026

Uh oh!

betinacosta Sep 15, 2026

Uh oh!

betinacosta commented Sep 15, 2026

Uh oh!

betinacosta commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return {"X-SubaccountId": subaccount_id} if subaccount_id else None


		def _quote_guid(value: str) -> str:

Uh oh!

Conversation

ShiwaliKusum commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Type of Change

How to Test

Checklist

Breaking Changes

Additional Notes

Uh oh!

cla-assistant Bot commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

NicoleMGomes commented Jul 1, 2026

Uh oh!

betinacosta Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

betinacosta Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

betinacosta commented Sep 15, 2026

Uh oh!

betinacosta commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ShiwaliKusum commented Jul 1, 2026 •

edited

Loading