Skip to content

test: first-pass unit coverage for tenant/identity/access/menu cores#9

Merged
jlc488 merged 8 commits into
mainfrom
test/coverage
May 26, 2026
Merged

test: first-pass unit coverage for tenant/identity/access/menu cores#9
jlc488 merged 8 commits into
mainfrom
test/coverage

Conversation

@jlc488

@jlc488 jlc488 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

5 small unit-test files covering the parts of the -core modules that are tricky enough to be worth isolated tests. No Spring context, no DB — pure JUnit + AssertJ.

Stacked on #3-#8.

What's covered

Module Test Covers
tenant-core FixedTenantResolverTest returns configured id repeatedly
tenant-core DefaultTenantContextHolderTest ThreadLocal set/get/clear round trip
identity-core BCryptPasswordHasherTest hash round trip, salt randomness, null-hash safety
access-core DefaultPolicyEvaluatorTest ALLOW / DENY / NOT_APPLICABLE paths, duplicate Policy.name() guard
menu-core MenuTreeBuilderTest root sort, parent/child grouping, required_permission_codeOptional<Permission>

Build infra

Root convention adds these testImplementation deps so all subprojects can use them:

  • org.assertj:assertj-core
  • org.mockito:mockito-core
  • org.mockito:mockito-junit-jupiter

Spring Boot BOM manages versions.

Verified

./gradlew test  → BUILD SUCCESSFUL in 1m 6s

(Includes the existing sample-app Testcontainers integration test.)

Out of scope (intentional, room for follow-ups)

  • @DataJpaTest per JPA repo — sample-app already exercises wiring at ApplicationContext level
  • WebMvc / MockMvc tests for admin-api controllers
  • End-to-end lockout scenario (5 BAD_CREDENTIALS → ACCOUNT_LOCKED → wait → auto-unlock) — needs a controllable Clock bean for time travel
  • HeaderTenantResolver / SubdomainTenantResolver with MockHttpServletRequest

jlc488 added 8 commits May 27, 2026 02:16
Two runtime-only blockers found by ./gradlew :devslab-kit-sample-app:test
(compile was always green, so neither showed up earlier):

1. Spring Data JPA AOT processor (`processTestAot`) failed with
   `IllegalArgumentException: MethodParameter.getParameterName() must not be null`
   when trying to generate AOT code for the @Param("userId") native query on
   JpaPlatformPermissionRepository.findCodesForUserId. Spring Data needs
   parameter names to survive into bytecode; the default javac in our
   subprojects {} convention was emitting them.
   Fix: add `options.compilerArgs.add("-parameters")` to
   `tasks.withType<JavaCompile>().configureEach` in the root convention.

2. ApplicationContext refresh failed with NoSuchBeanDefinitionException for
   ObjectMapper, requested by `auditLogService` in AuditAutoConfiguration.
   Spring Boot 4 didn't activate JacksonAutoConfiguration in our setup
   (likely because spring-boot-starter-webmvc no longer pulls it transitively
   the way starter-web did in 3.x). Rather than chasing the starter graph,
   register a `@ConditionalOnMissingBean ObjectMapper` fallback in
   AuditAutoConfiguration; consumer apps that already expose an ObjectMapper
   (e.g. via Jackson autoconfig) win via the conditional.

Verified: ./gradlew :devslab-kit-sample-app:test now passes (51s):
  - ApplicationContext boots cleanly
  - All 8 starter beans autowire (TenantResolver, TenantContextHolder,
    CurrentUserProvider, PasswordHasher, LocalLoginService,
    PermissionChecker, MenuProvider, AuditEventPublisher)
  - Flyway V1–V4 migrate against real Postgres (Testcontainers)
  - BCrypt round-trip works

Warning still observed (non-fatal, deferred): Spring Data Redis tries to
classify our JPA repositories as RedisHash candidates because we pull in
spring-data-redis. Will silence with `spring.data.redis.repositories.enabled=false`
in a follow-up.
…ator)

## Group (subject grouping)

Adds a User-Group-Role layer over the existing RBAC schema so that
permissions can be assigned to groups instead of (or in addition to)
individual users. Hierarchical groups via parent_group_id.

- core: GroupId value object
- access-api: Group record (id + tenantId + code + name + parentGroupId)
- access-core entities: PlatformGroupEntity, PlatformUserGroupEntity
  (user-group junction), PlatformGroupRoleEntity (group-role junction)
- access-core repos: JpaPlatformGroupRepository,
  JpaPlatformUserGroupRepository, JpaPlatformGroupRoleRepository
- access-core services: GroupService (CRUD + parent), GroupMembershipService
  (add/remove user + queries), GroupRoleService (assign/revoke role)
- DB: V5__platform_group.sql (FK cascade on group delete)
- DefaultPermissionChecker now resolves permissions via UNION of
  user_role + group_role -> role_permission (single native query in
  JpaPlatformPermissionRepository.findCodesForUserId)

## ABAC SPI (PolicyEvaluator)

Adds a pluggable, ABAC-style policy layer on top of RBAC. Consumer apps
register `Policy` beans with a name; PermissionChecker.isAllowed()
combines an RBAC check with an optional policy evaluation.

- access-api: policy/Policy interface, PolicyContext (record with
  user/tenant/resource/attributes/environment + Builder),
  PolicyDecision enum (ALLOW/DENY/NOT_APPLICABLE), PolicyEvaluator
  interface
- access-core: DefaultPolicyEvaluator (auto-registers all `Policy`
  beans by name; duplicate names throw)
- access-api: PermissionChecker gains
  `default isAllowed(Permission, String policyName, PolicyContext)`
  + `default check(Permission, String policyName, PolicyContext)`
  -- backward compatible, returns hasPermission() when policy ignored
- access-core: DefaultPermissionChecker overrides isAllowed to do
  RBAC then policy (DENY decision wins, NOT_APPLICABLE/ALLOW pass)
- AccessAutoConfiguration registers GroupService /
  GroupMembershipService / GroupRoleService / PolicyEvaluator,
  rewires DefaultPermissionChecker with new PolicyEvaluator dep

Deliberately NO DSL (Rego/Cedar). Consumers write `Policy` impls in
Java. External engines (OPA, AWS Cedar) belong in optional starters.

Verified: ./gradlew :devslab-kit-sample-app:test passes (1m, V5
migration applied, all beans wire, BCrypt round-trip still green).
…udit

New devslab-kit-admin-api module with 6 @RestController endpoints
covering the admin surface from planning §6.14 / §16.

## New module structure

devslab-kit-admin-api/
├─ AdminApiPaths            (constants: /admin/api/v1/...)
├─ ApiError                 (error response record)
├─ AdminApiExceptionHandler (@RestControllerAdvice for all admin endpoints)
└─ {user,role,permission,group,menu,audit}/
    ├─ *Controller         (REST)
    └─ *Request/Response   (DTOs with Jakarta Validation)

## Endpoints

| Resource | Verbs | Notes |
|---|---|---|
| /admin/api/v1/users | POST GET (id + list) PUT(lock/unlock/status/password) DELETE | tenant scoped via ?tenantId= |
| /admin/api/v1/roles | POST GET PUT DELETE + /{id}/permissions/{permissionId} grant/revoke + /{id}/users/{userId} assign/revoke | |
| /admin/api/v1/permissions | POST GET PUT DELETE | global (not tenant scoped, per planning) |
| /admin/api/v1/groups | POST GET PUT DELETE + /members/{userId} + /roles/{roleId} | |
| /admin/api/v1/menus | POST GET PUT DELETE | tenant scoped, parentId for hierarchy |
| /admin/api/v1/audit-logs | GET ?tenantId=&since= + /user/{userId} | |

## New admin service layer (in -core modules)

Thin services dedicated to admin write/lifecycle operations,
separate from the read services used by runtime:

- identity-core: PlatformUserAccountAdminService
  (create, lock/unlock, setStatus, resetPassword, delete, list)
- access-core: RoleAdminService, PermissionAdminService
- menu-core: MenuAdminService
- audit-core: AuditLogQueryService

Each gets a @ConditionalOnMissingBean factory in the new
AdminApiAutoConfiguration (registered in AutoConfiguration.imports),
ordered after the per-vertical auto-configs.

## Error handling

AdminApiExceptionHandler maps:
- AccountLoginException     -> 401
- PermissionDeniedException -> 403
- IllegalArgumentException  -> 400
- IllegalStateException     -> 409
- MethodArgumentNotValidException + ConstraintViolationException
  -> 400 with field details

Returns ApiError records with status/error/message/details/timestamp.

## Security note (deliberate first-pass)

These controllers do NOT call PermissionChecker themselves; consumer
apps wire a SecurityFilterChain (per planning §3 / starter
philosophy) and decide who can hit /admin/api/v1/**. A follow-up PR
can add an optional method-security pre-authorize starter.

## Verified

./gradlew :devslab-kit-sample-app:test  → BUILD SUCCESSFUL in 1m 2s
- AdminApiAutoConfiguration bean factories all wire
- 6 controllers picked up via @SpringBootApplication(scanBasePackages="kr.devslab.kit")
- previous 8 starter beans + V1–V5 migrations still green
Adds two new TenantResolver implementations beyond FixedTenantResolver,
selectable via devslab.kit.tenant.resolver property:

- HeaderTenantResolver: pulls tenant id from HTTP request header
  (default X-Tenant-Id, configurable via devslab.kit.tenant.header-name)
- SubdomainTenantResolver: pulls tenant id from request host name
  segment at a configurable index (default 0 — leftmost label of
  "acme.example.com" => tenant "acme")

Both fall back to devslab.kit.tenant.default-tenant-id when the request
doesn't carry tenant info (out-of-band lookups, scheduled jobs, etc.).
Throw IllegalStateException if neither source has a value.

TenantAutoConfiguration now switches on Resolver enum:
- FIXED      -> FixedTenantResolver (existing)
- HEADER     -> HeaderTenantResolver
- SUBDOMAIN  -> SubdomainTenantResolver
- JWT        -> IllegalStateException with explicit message pointing
                at the not-yet-shipped oauth2-resource-server starter
- CUSTOM     -> IllegalStateException telling consumer to provide
                their own TenantResolver bean (which @ConditionalOnMissingBean
                will then defer to)

DevslabKitProperties.Tenant gains:
- headerName (default "X-Tenant-Id")
- subdomainIndex (default 0)

tenant-core gets compileOnly deps on spring-web + jakarta.servlet-api
so the request-aware resolvers compile. They use
RequestContextHolder -> ServletRequestAttributes, so they're inert
outside an HTTP request (which is when the default-tenant-id fallback
matters).

Verified: ./gradlew :devslab-kit-sample-app:test  -> BUILD SUCCESSFUL in 1m 6s
(sample-app still uses FIXED, so the switch falls into the same branch
that was previously hardcoded; new resolvers covered by compile only here,
runtime test of HEADER/SUBDOMAIN comes with WebMvc integration tests in
the test-coverage PR.)
PlatformUserAccountEntity gains two columns + V6 migration:
- failed_login_count (int, NOT NULL DEFAULT 0)
- locked_until (timestamp with time zone, NULL = no scheduled unlock)

LocalLoginService now:
1. On entry, if account is locked AND locked_until has passed, auto-unlocks
   (failed_login_count reset to 0, lock cleared)
2. On BAD_CREDENTIALS, increments failed_login_count; once it hits
   maxFailedAttempts, sets locked=true and locked_until=now+lockoutDuration,
   then emits ACCOUNT_LOCKED rather than BAD_CREDENTIALS on the event
3. On success, if there were any prior failures, resets failed_login_count
   to 0 and clears locked_until (defensive)

Changed @transactional(readOnly = true) -> @transactional because we
now mutate entity state on every login (counter or reset).

DevslabKitProperties.Identity adds:
- maxFailedAttempts (default 5)
- lockoutDuration (default 15m, java.time.Duration so YAML can use
  "PT30M" / "5m" syntax via Spring's relaxed binding)

IdentityAutoConfiguration's LocalLoginService bean now takes
DevslabKitProperties so the two new knobs flow through. Existing
@ConditionalOnMissingBean still lets consumers swap the whole service.

Verified: ./gradlew :devslab-kit-sample-app:test  -> BUILD SUCCESSFUL in 57s
(V6 migration applies cleanly on top of V1-V5; previous beans still wire.
End-to-end lockout behavior covered by unit tests in the test-coverage PR.)
.github/workflows/build.yml runs on every push to main and every PR
targeting main. Single job:

1. Checks out the repo
2. Installs GraalVM CE 21 via graalvm/setup-graalvm@v1 (matches the
   project toolchain so nativeCompile can be added later without
   another JDK install)
3. Sets up Gradle wrapper caching (gradle/actions/setup-gradle@v4)
4. Runs `./gradlew build --no-daemon --stacktrace`
   - This triggers ./gradlew :devslab-kit-sample-app:test which uses
     Testcontainers. ubuntu-latest has Docker available, so
     Testcontainers spins Postgres + Redis on demand. No
     GitHub Actions `services:` block needed (would conflict with
     Testcontainers' own port allocation).
5. On failure, uploads test reports (HTML + XML) as an artifact for
   debugging — kept 7 days.

`concurrency: cancel-in-progress: true` so pushing a new commit to a
PR branch cancels the previous run instead of stacking them.

Once this merges, all open PRs (#3-#7) will get an automatic green/
red signal next time CI runs.

Not in this PR (follow-ups):
- nativeCompile job (matrix add-on, optional / tag-only)
- Release workflow (Maven Central publish, separate trigger)
- CodeQL / dependency-review (security scan)
Adds 5 small unit-test files (no Spring context, no DB) covering the
parts of the -core modules that are tricky enough to be worth
isolated tests:

- tenant-core
  * FixedTenantResolverTest        : returns configured id repeatedly
  * DefaultTenantContextHolderTest : ThreadLocal set/get/clear round trip
- identity-core
  * BCryptPasswordHasherTest       : round trip, salt randomness, null hash
- access-core
  * DefaultPolicyEvaluatorTest     : ALLOW/DENY/NOT_APPLICABLE paths,
                                     duplicate Policy-name guard
- menu-core
  * MenuTreeBuilderTest            : root sort, parent/child grouping,
                                     required_permission_code -> Optional<Permission>

Root convention adds AssertJ + Mockito (with junit-jupiter integration)
as testImplementation so all subprojects can use them without per-module
boilerplate. Spring Boot BOM manages the versions.

Verified: ./gradlew test  -> BUILD SUCCESSFUL in 1m 6s
(includes the existing sample-app Testcontainers integration test.)

Out of scope here (intentional, room for follow-ups):
- @DataJpaTest coverage for the JPA repos (Spring + Testcontainers per
  module is expensive; sample-app already exercises the wiring at the
  ApplicationContext level)
- WebMvc / MockMvc tests for the admin-api controllers
- End-to-end lockout scenario test (5 BAD_CREDENTIALS -> ACCOUNT_LOCKED ->
  wait -> auto-unlock) — needs a controlled Clock bean injected into
  LocalLoginService for time-travel
- HeaderTenantResolver / SubdomainTenantResolver tests with MockHttpServletRequest
Same fix as PR #8 — Windows commits lose the executable bit, Linux
CI then fails with 'Permission denied (exit 126)'. Once PR #8 merges
into main this commit becomes redundant and can be dropped via rebase.
@jlc488
jlc488 merged commit 49c31f1 into main May 26, 2026
1 check passed
@jlc488
jlc488 deleted the test/coverage branch May 30, 2026 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant