UNOMI-139 UNOMI-880 UNOMI-878 UNOMI-884: Multi-tenant platform core#760
Merged
Conversation
1 task
sergehuber
changed the title
This was referenced May 19, 2026
sergehuber
force-pushed
the
UNOMI-139-platform
branch
from
4c5828e to
d2093d1
Compare
sergehuber
changed the base branch from
UNOMI-888-import-export-javadoc
to
master
sergehuber
force-pushed
the
UNOMI-139-platform
branch
from
d2093d1 to
6f3f768
Compare
Log the full event chain once per collapsed stack pattern instead of on every blocked send(), avoiding log storms during rule recursion loops.
Add no-arg constructor and setUpdated() so clients can deserialize POST /cxs/eventcollector JSON (aligned with unomi-3-dev, without tracing fields).
sergehuber
force-pushed
the
UNOMI-139-platform
branch
from
6f3f768 to
c36e1fd
Compare
Increase retry budget from 10 to 20 for the property type removal poll. ES consistently takes ~11s to propagate the deletion, exceeding the 10s limit.
sergehuber
commented
Jun 3, 2026
sergehuber
left a comment
sergehuber
left a comment
Contributor
Author
There was a problem hiding this comment.
Code Review: UNOMI-139 Multi-tenant platform core
Scope: 292 files, +30 486 / −4 093 lines (multi-tenancy, unified caching, cluster scheduling, V3 migration).
| # | Severity | Finding |
|---|---|---|
| C1 | CRITICAL | hasTenantAccess() ignores tenantId — any tenant key accesses all tenants |
| C2 | CRITICAL | Unknown X-Unomi-Tenant-Id header grants full system context to JAAS users |
| C3 | CRITICAL | Private API key committed in plaintext to migration script (permanently in git history) |
| C4 | CRITICAL | ApiKey.key serialised to ES and returned by REST with no @JsonIgnore |
| H1 | HIGH | executeAsSystem() finally block suppresses original exception on dual failure |
| H2 | HIGH | Migration step key uses itemType — multiple rollover indices silently skipped |
| I1 | IMPORTANT | SecurityService.SYSTEM_TENANT = "SYSTEM_TENANT" conflicts with all other constants ("system") |
| I2 | IMPORTANT | isPublicOperation() throws IndexOutOfBoundsException on empty/fragment-only documents |
| I3 | IMPORTANT | migrate-3.1.0-00-tenantDocumentIds.groovy conflicts with existing 00-fixProfileNbOfVisits on master |
| I4 | IMPORTANT | ScheduledTask implements Serializable without serialVersionUID |
| I5 | IMPORTANT | getCurrentRoles() is dead code calling AccessController removed in Java 21 |
| I6 | IMPORTANT | isOperatingOnSystemTenant() stub always returns false |
| A1 | Advisory | @author tag in SchedulerServiceImpl (ASF policy) |
All findings ≥ 80 % confidence. Inline suggestions attached. C1–C4 should be resolved before merge.
…d log4j2 improvements Fix several issues raised in PR #760 review: - [I1] Fix SYSTEM_TENANT constant value ("SYSTEM_TENANT" → "system") so system tenant context resolution works correctly across all services - [I4] Add missing serialVersionUID to ScheduledTask to prevent deserialization issues across cluster nodes - Fix PatchIT.testRemove flaky timeout: polling for PropertyType removal must run inside executeAsSystem() context. Because system PropertyTypes are inherited by other tenants, polling outside the system tenant context would still observe the item through inheritance after deletion. - Fix cache refresh race condition in AbstractMultiTypeCachingService: scheduled and manual refreshes were allowed to run concurrently (allowParallelExecution defaults to true in the scheduler). Add disallowParallelExecution() to prevent a scheduled refresh from re-populating the cache with stale data during a manual removal. - Add comprehensive unit tests for keepTrying(), waitForNullValue() and shouldBeTrueUntilEnd() polling utilities in PollingUtilsTest (558 lines, 47 tests, 100% coverage) - Fix infinite loop bug in keepTrying() when predicate is satisfied on a null value (do-while pattern ensures predicate is tested after fetch) - Improve log4j2 configuration: - Separate patterns for console (colors) and file (plain text) - Configurable message truncation via org.apache.unomi.logs.message.max.length (defaults to 500 chars, security feature against log injection) - Shorter bundle info (B <id> instead of id/name/version) - Thread name truncated from beginning (%-16.16t) to preserve the significant suffix - Disable all SafeExtendedThrowablePatternConverter transformations via --disable-log-truncation flag in build.sh for debugging - Add build.sh flags: --disable-log-truncation, --maven-quiet, --search-heap, --karaf-heap
…safety issues - ExecutionContext.setTenant/restorePreviousTenant: recompute isSystem from tenantId so a system context cannot persist after switching to a tenant (SEC-3) - TaskStateManager: retryDelay is in milliseconds, remove erroneous toMillis() conversion that produced multi-year retry delays (UNOMI-939) - TaskExecutionManager: remove "Simulated crash" sentinel that swallowed real exceptions and left tasks permanently locked (UNOMI-939) - TenantQuotaService.checkQuota: null guard for unconfigured quota avoids NPE on new tenants (UNOMI-942) - AuditServiceImpl.getModifiedItems: fill empty if-body so a null persistenceService returns early instead of falling through to NPE
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.
Stacked PR (merge order)
UNOMI-888-import-export-javadocUNOMI-139-platformThis PR is stacked: merge into
UNOMI-888-import-export-javadocfirst (PR #756, UNOMI-888), after that line is merged or rebased. Downstream PRs (UNOMI-904 V2 compat, dev shell, additional ITs, manual) will stack onUNOMI-139-platform.For more information (each PR targets the branch below until the bottom merges): https://github.github.com/gh-stack/introduction/overview/
JIRA
Summary
Delivers the Unomi 3.1 platform core on top of UNOMI-888: multi-tenant execution, unified caching, cluster-aware scheduling, and V3 migration assets. Services, persistence, GraphQL, and REST are wired for V3 tenant resolution (API keys / roles).
Supersedes the platform scope of PR #757 (closed; monolith branch
UNOMI-139-UNOMI-880-multitenancy). UNOMI-904 (V2 compatibility), dev shell commands, additional IT classes, and manual updates will follow in stacked PRs on this line.UNOMI-139 Multi-tenancy
Tenant,TenantService, API keys, security/audit interfaces, quotas, lifecycle hooks) andtenantIdon core types.ExecutionContextManagerand thread-local tenant context so services, persistence, and REST resolve the active tenant consistently.UNOMI-880 Unified multi-tenant caching
MultiTypeCacheService/CacheableTypeConfigandAbstractMultiTypeCachingServicefor shared cache lifecycle, predefined item bundling, periodic refresh, and statistics.DefinitionsServiceImplandSegmentServiceImpl(and related paths) onto the unified cache; remove duplicated ad-hoc cache listeners where superseded.UNOMI-878 Cluster-aware task scheduling
SchedulerService(scheduled tasks, task executors, persistence-backed vs in-memory tasks, cluster coordination).UNOMI-884 Migration to V3
MigrationUtilsandMigrationUtilsTestfor the new steps aligned with multi-tenancy and index updates.Tests
BaseIT, migration ITs,ScopeIT, and related suites)../build.sh --integration-tests --use-opensearch).Licence