feat(adms): add SAP ADMS module with sync/async clients and BDD tests#138
Merged
Conversation
|
|
Adds a full-featured ADMS (Attachment Document Management Service) module to the SDK with sync and async clients, IAS X.509 token authentication, OData V4 service support (DocumentService, ConfigurationService, AdminService), and pytest-bdd integration tests with Gherkin scenarios. Also adds shared SDK building blocks consumed by ADMS: IAS token fetcher, mTLS support, async HTTP client, and the ADMS telemetry module entry.
Required by upstream's "Enforce version bump when src/ is modified" CI check.
- Revert global [tool.pytest.ini_options] integration marker description and remove asyncio_mode=auto so the change does not leak into other modules' test runs. - Extract /etc/secrets/appfnd and CLOUD_SDK_CFG as module-level constants in adms/config.py for consistency with the existing _SERVICE_PATH / _ADMIN_SERVICE_PATH constants.
Upstream's data-anonymization PR (SAP#93) also bumped to 0.20.0 — bumping to 0.20.1 to satisfy the version-bump CI check on the rebased branch.
adwitiyasushant
force-pushed
the
contrib/adms
branch
from
2a3f60e to
74a9909
Compare
Required for pytest-asyncio strict mode (the project default after asyncio_mode auto was scoped out). Matches the convention already in use in tests/agentgateway/.
…trings Leftover wording from the dms module template — exception, config, auth and http docstrings now correctly identify themselves as ADMS. No behavioural change.
ArthurTonial
requested changes
May 27, 2026
- Add ADMS placeholder block to .env_integration_tests.example so reviewers know which env vars to set for external/BTP mode. - Correct the env var names in conftest docstring and INTEGRATION_TESTS_ADMS.md: the loader expects CLIENTID / CLIENTSECRET / URL / URI (matching the IAS binding field names used by destination/), not the underscored CLIENT_ID / IAS_URL / SERVICE_URL variants — those would have failed with KeyError. - Add the optional RESOURCE entry (IAS resource URI) the docs were missing. - Rename leftover "DMS Integration Tests" heading to "ADMS Integration Tests".
- Remove unused re-exports from adms/_auth.py (_CC_CACHE_KEY, _GRANT_JWT_BEARER) - Make InMemoryTokenCache thread-safe via threading.Lock - Drop module-specific reference from core/http async client docstring - Delete unused core/http/_batch.py and its tests/BDD scenarios - Remove 5 pattern YAML stubs from docs/adms/patterns/ - Restore underscore names for internal API classes in adms unit tests
Per PR review: integration tests target real service instances, so the ADMS-specific guide duplicates the canonical doc. Merge env vars into docs/INTEGRATION_TESTS.md and delete docs/INTEGRATION_TESTS_ADMS.md.
Per PR review. Local-only test driver — kept on developer machines via .gitignore but no longer shipped with the SDK.
… pattern Per PR review: - Drop the special CLOUD_SDK_ADMS_INTEGRATION_URL block from the workflow. ADMS now flows through the existing CLOUD_SDK_CFG_* loader like every other module. - Rewrite tests/adms/integration/conftest.py to target real ADM instances only. Removes the local HDM Maven auto-start mode (~150 lines), the mock IasTokenFetcher, and the CLOUD_SDK_ADMS_SKIP_IF_UNAVAILABLE flag. - Skip the suite gracefully when load_from_env_or_mount() raises ConfigError because of missing credentials. - Update docs and .env_integration_tests.example to match.
The concurrent-create integration test built coroutines and called asyncio.gather() from sync code, binding the resulting future to the default event loop. The run_async fixture then ran it in a separate loop, raising "future belongs to a different loop" against a real ADM instance. Wrap the gather in an async helper, mirroring the existing cleanup_concurrent_async_relations pattern, so coroutines and gather share the run_async fixture's loop.
…example Per PR review. The 'resource' field example pointed to a personal IAS application name; replace with the generic 'my-adm-app'.
Per PR review. Replace internal-only "Unified Provisioning / UCL" and "BTP Fabric SDK Business Services TRA" references with the generic "BTP service instance" wording suitable for a public SDK. Also fix the stale "DMS" tag on the first line; the module is ADMS.
Per PR review. The defensive untyped parameter and "avoid circular import" comment were incorrect — config.py imports only from core and exceptions, no cycle exists. Add the AdmsConfig import and annotate the parameter properly.
Per PR review. HttpError was imported twice — once bare and once aliased as AdmsHttpError. Since CoreHttpError is already aliased on the line below, the bare HttpError name is unambiguous. Drop the duplicate alias and use HttpError consistently.
Per PR review.
Per PR review. Replace `from sap_cloud_sdk.core.auth._token_cache import TokenCache` with the public re-export `from sap_cloud_sdk.core.auth import TokenCache` in client.py and __init__.py to avoid reaching into the private `_token_cache` module.
- Expand "ADM Document Management Service" to "Advanced Document Management Service" in _models.py docstring (consistent with the rest of the module). - Drop SAP-internal "Unified Provisioning / UCL" wording in user-guide.md; use the public "BTP service instance" phrasing already used in __init__.py.
Per PR review. * PEP 8: rename `mTLSConfig` → `MTLSConfig` and `mTLSStrategy` → `MTLSStrategy`. Public-facing classes now follow the standard PascalCase rule. All callers, BDD scenarios, step definitions, error messages and test class names are updated. * Drop the unused `client: Optional[httpx.AsyncClient] = None` parameter from `MTLSStrategy.apply_to_async_client`. The previous signature documented the argument as "Ignored (kept for API symmetry)" — it was never honoured because httpx does not allow swapping the SSL context on an existing client. The misleading parameter is removed and the docstring rewritten accordingly. Module docstring example and BDD step are updated to call without the argument.
Per PR review. `_CC_CACHE_KEY` and `_GRANT_JWT_BEARER` are private constants of `_ias_fetcher` and should not appear in the public `sap_cloud_sdk.core.auth` namespace. Drop them from the import list and `__all__` in core/auth/__init__.py, and update tests/adms/unit/test_auth.py to import `_CC_CACHE_KEY` directly from the private module (matching the existing pattern in tests/core/unit/auth/test_ias_fetcher.py).
# Conflicts: # pyproject.toml # uv.lock
* tests/adms/integration/test_e2e_async_flow.py — bind narrowed context.client / context.bo_type_id to local variables before the nested `_gather` async closure. ty re-widens optional attributes inside nested closures, so the outer-function asserts no longer apply once the closure references them. Local-variable binding preserves narrowing. * pyproject.toml + uv.lock — bump version 0.21.1 → 0.22.0. Required by check-version-bump CI now that the merge with main has introduced src/ changes (new ADMS module). Minor bump because this adds a new optional module.
betinacosta
reviewed
May 29, 2026
The stray removal of the # ty: ignore[...] comment in tests/dms/integration/conftest.py was the same out-of-scope edit pattern NicoleMGomes flagged on objectstore/agent_memory/destination conftests. Restore the upstream version — DMS module changes belong in a separate PR.
Pre-existing unused imports in test_async_http.py (MagicMock, patch) and test_models.py (pytest, JobInput, JobType). Caught by ruff's F401 lint rule; auto-fixed with --fix.
…ument The Document entity set is not exposed as a top-level queryable collection on the ADM service — GET /DocumentService/Document returns HTTP 405 on every real tenant. Documents are only reachable as children of DocumentRelation. Fix: get_all() now transparently queries DocumentRelation with $expand=Document and returns the deduplicated set of Document objects (first-seen order, keyed by DocumentID). Caller-supplied $filter / $top / $skip / $orderby are forwarded to the DocumentRelation query. If the caller's options already include an $expand list, Document is merged into it rather than replacing it. Async variant updated identically. Unit tests replaced to cover: - happy-path return via relation payload - relations without a Document are skipped (null navigation) - duplicate documents (same DocumentID on 2 relations) deduplicated - $expand always contains "Document" - caller's extra $expand values are merged, not overwritten - remaining query params ($filter, $top, $skip, $orderby, $select) forwarded - entity-set queried is DocumentRelation, not Document
1. _document_api.py: type rel_params as dict[str, str | int] (not bare dict) and wrap the get() result in str() before calling .split() — ty couldn't prove the value was str when the dict held int values. Applies to both sync and async get_all. 2. tests/dms/integration: remove two now-unused # ty: ignore directives (conftest.py:17, test_dms_bdd.py:131) that ty 0.25 no longer needs. These were flagged as warnings-turned-errors after the upstream merge bumped ty. 3. pyproject.toml / uv.lock: bump version 0.25.0 → 0.25.1 to satisfy the CI version-bump gate (src/ files changed, version must increase).
…esponse DocumentRelation.from_dict was silently dropping 7 fields that the ADM service returns (confirmed against live Postman/Bruno response): is_active_entity (IsActiveEntity) has_active_entity (HasActiveEntity) has_draft_entity (HasDraftEntity) document_relation_is_output_relevant (DocumentRelationIsOutputRelevant) draft_messages (DraftMessages) doc_relation_changed_by_user_name (DocRelationChangedByUserName) doc_relation_changed_at_date_time (DocRelationChangedAtDateTime) CLI output now matches the wire response exactly instead of showing a truncated view.
…dels
_http.py:
- HttpError now includes the ADM error message extracted from the OData
error body ({"error": {"message": "..."}}) in the exception message,
so callers see e.g. "ADMS service returned HTTP 409 — Document is locked"
instead of the bare status code.
_models.py — 4 config entity models were missing audit fields returned by
the live API:
AllowedDomain:
+ created_by_user_name (CreatedByUserName)
+ created_at_date_time (CreatedAtDateTime)
+ changed_by_user_name (ChangedByUserName)
+ changed_at_date_time (ChangedAtDateTime)
DocumentType: same 4 fields
BusinessObjectNodeType: same 4 fields
DocumentTypeBusinessObjectTypeMap: same 4 fields
All from_dict() methods updated to parse the new fields; to_odata_dict()
methods unchanged (audit fields are server-set, not sent on write).
The draft lifecycle actions (CreateBusinessObjNodeDraft, ValidateBusinessObjNodeDraft, ActivateBusinessObjNodeDraft) return a DraftAdministrativeData nested object on each DocumentRelation. This was silently dropped by from_dict() — now modelled as a first-class dataclass and parsed into draft_administrative_data on DocumentRelation. Fields: draft_uuid, creation_date_time, created_by_user, draft_is_created_by_me, last_change_date_time, last_changed_by_user, in_process_by_user, draft_is_processed_by_me. Re-exported in adms/__init__.py so callers can type-hint against it.
AllowedDomain, DocumentType, BusinessObjectNodeType, and DocumentTypeBusinessObjectTypeMap were each missing two operations visible in the live API (GET by key, PATCH): config.get_allowed_domain(id) → GET AllowedDomain(id) config.update_allowed_domain(id, input) → PATCH AllowedDomain(id) config.get_document_type(id) config.update_document_type(id, input) config.get_business_object_type(id) config.update_business_object_type(id, input) config.get_type_mapping(id) → GET TypeMap(id) All 8 methods have sync + async variants. Three new Update*Input dataclasses model the PATCH payload (only non-None fields sent). Eight new Operation enum entries; total adms operations 33 → 40 → 131.
… ContentHash
_document_api.py:
- update() and its async twin now POST UpdateDocument then follow up
with GET .../Document to return the complete entity.
ADM's UpdateDocument action only returns changed fields; the extra
GET costs one round-trip but gives the caller the same full response
as Postman shows (all fields populated, not mostly null).
_models.py:
- Document gains two fields present in the Postman response but
previously silently dropped:
document_state_text (DocumentStateText) — e.g. "File Extension Restricted"
document_content_hash (DocumentContentHash) — SHA-256 of uploaded content
… fields
Four config models had CreatedBy/ChangedBy audit fields that do not exist
in the ADM CDS schema and are never returned by the service (confirmed
against live Postman response). Those four null fields have been removed:
AllowedDomain — removed 4 audit fields
DocumentType — removed 4 audit fields
BusinessObjectNodeType — removed 4 audit fields; added 2
missing CDS fields: ODMEntityName,
ApplicationTenantID
DocumentTypeBusinessObjectTypeMap — removed 4 audit fields
CLI _WIRE_KEYS updated accordingly.
SDK additions (sync + async for all):
DocumentService (_relation_api.py):
- delete_business_object_node(DraftInput) → DeleteBusinessObjectNodeResult
Deletes ALL relations for a BO node; requires system-user scope.
- get_change_logs() → list[ChangeLog]
Read-only audit trail for all document management changes.
- get_bo_node_change_logs() → list[BusinessObjectNodeChangeLog]
Change log joined with BO node / document context.
ConfigurationService (_configuration_api.py):
- mark_default(mapping_id) — sets a DocType↔BOType map as default
- FileExtensionPolicy CRUD:
get_all_file_extension_policies, create_file_extension_policy,
get_file_extension_policy, delete_file_extension_policy
- ApplicationTenant CRUD:
get_all_application_tenants, create_application_tenant,
get_application_tenant, delete_application_tenant
New models (_models.py):
ChangeLog, BusinessObjectNodeChangeLog, DeleteBusinessObjectNodeResult,
FileExtensionPolicy, CreateFileExtensionPolicyInput, MimeTypePolicy,
ApplicationTenant, CreateApplicationTenantInput
12 new Operation enum entries; total 131 → 143.
CLI (scripts/adms_cli.py — local only):
rfu full upload: generate URL + PUT file to GCS + complete in one step
rbn deleteBusinessObjectNode [irreversible]
rcl getChangeLog
rbl getBusinessObjectNodeChangeLog
cmk markDefault (mapping)
cfl/cfg/cfc/cfd FileExtensionPolicy list/get/create/delete
cal/cag/cac/cad ApplicationTenant list/get/create/delete
The CDS entity uses 'BusinessObjectNodeType' (not 'BusinessObjectNodeTypeID') for the short identifier field. The SDK was sending 'BusinessObjectNodeTypeID' which ADM rejected with HTTP 400. BusinessObjectNodeType model: - business_object_node_type_id → business_object_node_type - removed business_object_type_id (not in CDS) - from_dict reads "BusinessObjectNodeType" (not "BusinessObjectNodeTypeID") - to_odata_dict writes "BusinessObjectNodeType" CreateBusinessObjectNodeTypeInput: - business_object_node_type_id → business_object_node_type - removed business_object_type_id - added application_tenant_id (required by ADM — was missing) UpdateBusinessObjectNodeTypeInput: - added business_object_node_type as patchable field Tests and CLI updated accordingly.
NicoleMGomes
previously approved these changes
Jun 10, 2026
ArthurTonial
previously approved these changes
Jun 11, 2026
1. _http.py — AsyncAdmsHttp.get/post/delete/patch now accept the base-class headers/content params (ignored internally) so the overrides are LSP- compliant. Removed the # type: ignore[override] suppressions that the new ty no longer accepts. 2. tests/adms/unit/test_client.py — MagicMock assignments to method slots annotated with # type: ignore[assignment, method-assign] to match the new ty rule name (was [method-assign] only). 3. tests/adms/unit/test_http.py — quote_odata_guid_key(None) annotated with # type: ignore[arg-type, argument-type] for new ty rule name. 4. tests/adms/unit/test_query_options.py — RelationQueryOptions(orderby=…) annotated with # type: ignore[call-arg, unknown-argument]. 5. tests/adms/unit/test_models.py — ruff format applied.
ty uses its own comment syntax (# ty: ignore[rule]) distinct from mypy's (# type: ignore[rule]). The previous commit used mypy syntax which ty ignores, leaving the errors unsuppressed. Affected lines: test_client.py:91-92 invalid-assignment (MagicMock on method slots) test_client.py:426 invalid-assignment (MagicMock on with_user_jwt) test_client.py:432 unresolved-attribute (assert_called_once_with) test_http.py:274 invalid-argument-type (quote_odata_guid_key(None)) test_query_options.py:86 unknown-argument (orderby on RelationQueryOptions) .gitignore: restore ADMS-specific entries (.env.adms, scripts/adms_cli.py, .DS_Store, .ucl-provision/) that were dropped during the upstream rebase. Also adds the trailing newline that end-of-file-fixer requires.
DocumentSizeInByte and DocContentVersionSizeInByte are Decimal(12) in the CDS — whole-number byte counts. float allows fractional values that can never occur and loses precision for files > 2^53 bytes. Affected fields: Document.document_size_in_byte: float | None → int | None DocumentContentVersion.doc_content_version_size_in_byte: float | None → int | None Test fixture updated: DocumentSizeInByte: 1024.0 → 1024.
Per reviewer feedback (NicoleMGomes): - Remove .ucl-provision/ and src/sap_cloud_sdk/adms/ucl/ — UCL provisioning artefacts belong to a separate repo, not the SDK. - Remove scripts/adms_cli.py — local debug script that only exists on the author's machine; no scripts/ directory is tracked in this repo. Kept: - .DS_Store — standard macOS metadata guard - .env.adms — any contributor following the ADMS integration test setup creates this file locally; the entry prevents accidental commits of IAS credentials.
The existing .env* wildcard on line 10 already ignores .env.adms. The explicit entry is redundant and was correctly questioned by the reviewer.
ArthurTonial
previously approved these changes
Jun 11, 2026
ArthurTonial
approved these changes
Jun 11, 2026
NicoleMGomes
approved these changes
Jun 11, 2026
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
4 participants
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.
Description
Adds the ADMS (Advanced Document Management Service) module to the
SAP Cloud SDK for Python.
Core components
AdmsClient(sync) andAsyncAdmsClient(async) with sub-namespacesfor
documents,relations,jobs, and configuration readscreate_client()/create_async_client()factories that load IASbindings from a mounted secret volume with environment-variable fallback
X509_GENERATED) client-credentials authentication, withoptional
resourcescoping for ADM application audience bindingDocumentService,AdminService, andConfigurationServicepathsFeatures
(
PENDING/CLEAN/INFECTED)DocumentRelationlifecycle including draft create / validate /activate / discard
business-object node types
ZIP_DOWNLOADandDELETE_USER_DATAjob orchestration viaAdminServiceSupporting infrastructure (in
core/)IasTokenFetcher—client_credentials(cached) andjwt-beareron-behalf-of (not cached, by design) grant flows
AdmsHttp/AsyncAdmsHttp— typed HTTP helpers with OData errormapping, X-CSRF-Token fetch-and-carry (per service root), and
thread- / coroutine-safe CSRF caching
MTLSStrategy— X.509 mTLS for services usingaccessStrategy: sap:cmp-mtls:v1, with explicitclose()andcontext-manager lifecycle
TokenCacheinterface withInMemoryTokenCacheandRedisTokenCache(multi-pod thundering-herd mitigation)py.typedmarkers oncore.auth,core.http,admsRelated Issue
N/A — this is a new module addition with no tracked issue.
Type of Change
How to Test
Expected: all unit tests pass.
uv run ruff check src/sap_cloud_sdk/
uv run ty check --python-version 3.11 src/
Expected: clean.
Set the standard ADMS env vars (or mount the secret volume):
CLOUD_SDK_CFG_ADMS_DEFAULT_URL (IAS tenant URL)
CLOUD_SDK_CFG_ADMS_DEFAULT_URI (ADM service URL)
CLOUD_SDK_CFG_ADMS_DEFAULT_CLIENTID
CLOUD_SDK_CFG_ADMS_DEFAULT_CLIENTSECRET
CLOUD_SDK_CFG_ADMS_DEFAULT_RESOURCE (optional)
Then:
uv run python -m pytest tests/adms/integration/ -m integration -v
Expected: all 17 BDD scenarios pass — sync + async document/relation
CRUD, draft flow, scan-state, concurrent creates, and 404 paths.
Checklist
Breaking Changes
None. This PR is purely additive — it introduces a new adms package and adds helpers under core.auth / core.http without modifying any existing public API.
Additional Notes