Document AI: Multitenancy#122
Conversation
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR successfully implements multi-tenancy for the Document AI plugin with solid test coverage across unit and integration layers. There are a few issues to address: the unsubscribe filter uses a denylist instead of an allowlist for active job statuses (fragile against new statuses), tenant IDs are appended to URLs without URI encoding (potential malformed requests), a test verification pattern incorrectly invokes the mock under verify() (unreliable assertion), and the failure result event emission doesn't guard against a null jobId.
PR Bot Information
Version: 1.28.2
- Event Trigger:
pull_request.opened - LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
efd3a6a0-84d2-11f1-8c4d-0b4c74e387a8 - File Content Strategy: Full file content
| } | ||
| } | ||
|
|
||
| static boolean detectMultiTenancy(CdsRuntime runtime) { |
There was a problem hiding this comment.
The exact same function exists in AICoreServiceConfiguration.java - since this is only one function I'd leave this as it is, but in case there is more shared code at some point, this might be extracted into a shared internal utility module (e.g. cds-ai-common).
| if (result.status() == ExtractionResult.Status.FAILED) { | ||
| logger.error("[sap-document-ai] Extraction failed for fileName={}", event.getFileName()); | ||
| if (result.internalJobId() != null) { | ||
| emitExtractionResult(context, result.internalJobId(), null); |
There was a problem hiding this comment.
We emit a DocumentExtractionResult when the extraction failed?
How does the consumer know this failed?
Can we set a status field in DocumentExtractionResult?
|
|
||
| @Before | ||
| @HandlerOrder(HandlerOrder.EARLY) | ||
| public void beforeUnsubscribe(UnsubscribeEventContext context) { |
There was a problem hiding this comment.
So: a tenant unsubscribes, before that actually is executed we mark all his jobs as failed, right?
Why do we not do this in afterUnsubscribe? That way, in case unsubscription fails for some reason, we have not marked the jobs as failed before.
| String path = DOCUMENT_AI_API_PATH + DOCUMENT_JOBS + "/" + dieJobId + EXTRACTED_VALUES_TRUE; | ||
| if (tenantId != null) { | ||
| path = | ||
| path + "&" + CLIENT_ID_PARAM + "=" + URLEncoder.encode(tenantId, StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Can we also use buildUriWithClientId here?
|
|
||
| private void processJobsForTenant(String tenantId, List<ExtractionJob> jobs) { | ||
| if (tenantId == null) { | ||
| jobs.forEach(job -> processJob(job, null)); |
There was a problem hiding this comment.
A job without a tenant id is processed in a MT-scenario: How can this actually happen?
In case there is a good reason for this: Add a warning here: multitenancy enabled but processing jobs without tenant ID.
|
Also: as of now there is no real integration test connecting to a real SAP Document Information Extraction instance - the tests in cds-feature-ai-core use a real AI Core instance. Possibly we can add a document ai instance to the same cf space and test the document extraction end-to-end? |
2 participants
Document AI: Multi-Tenancy Support for SAP CAP Java Plugin
New Features
✨ Implements full multi-tenancy support for the SAP Document AI CAP Java plugin. Each subscriber tenant's documents are now fully isolated within the shared Document AI service instance. Multi-tenancy is detected automatically at startup — no configuration flag is required.
Key additions:
DocumentAiSetupHandler: New handler registered only in multi-tenant mode. Hooks into the CAP MTXDeploymentServicelifecycle — logs tenant onboarding on subscribe, and marks all active jobs (PENDING,SUBMITTED,RUNNING) asFAILEDon unsubscribe to prevent poll errors after credentials are revoked.ExtractionPollingHandlernow groups active jobs bytenantIdand switches the CDS request context per group viaruntime.requestContext().systemUser(tenantId).run(...).clientIdquery parameter: All Document AI API calls now append?clientId=<tenantId>(submit) and&clientId=<tenantId>(poll) when a tenant ID is present, enabling job isolation within the shared service instance.DocumentSubmissionHandlernow emits aDocumentExtractionResultevent immediately when submission fails.isTerminal()helper: Added toExtractionStatusenum to simplify terminal state checks.Changes
DocumentAiSetupHandler.java: New file. Hooks into MTX subscribe/unsubscribe lifecycle. Marks active tenant jobs asFAILEDon unsubscribe.DocumentAiServiceConfiguration.java: AddeddetectMultiTenancy()method; conditionally registersDocumentAiSetupHandler; passesmultiTenancyEnabledflag toExtractionPollingHandler.ExtractionPollingHandler.java: AddedmultiTenancyEnabledflag andprocessJobsForTenant()method; tenant ID is now passed through to the HTTP client on each poll.DefaultDocumentAiClient.java/DocumentAiClient.java:submitDocumentandgetJobResultnow accept atenantIdparameter;clientIdquery param appended when non-null.DefaultDocumentAiProcessingService.java/DocumentAiProcessingService.java:processDocumentinterface and implementation updated to propagatetenantId.DocumentSubmissionHandler.java: EmitsDocumentExtractionResultevent on submission failure.ExtractionStatus.java: AddedisTerminal()utility method.DocumentAiSetupHandlerTest.java(unit + integration): New test files covering subscribe logging, unsubscribe job marking, and tenant isolation.ExtractionPollingHandlerTest.java: Added tests for per-tenant context switching and single-tenant bypass.DefaultDocumentAiClientTest.java: Added tests verifyingclientIdquery param presence/absence.README.md,architecture.md,handover.md: Documentation updated to reflect multi-tenancy as implemented; removed "future work" language; updated capability tables and handler descriptions.samples/bookshop/app/index.html: Updated UI5/Fiori sandbox script URLs to pinned version1.148.2.AbstractDocumentAiTest.java: Updated test polling client signatures to match newtenantIdparameters.PR Bot Information
Version:
1.28.2anthropic--claude-4.6-sonnet60edba20-84dc-11f1-8c27-ce5a0ab59a0bpull_request.edited