DEV Community

Cover image for Testing the Service Layer - Part 2: Where the Shared Ancestor Ends (Chapter 10)
Kamen
Kamen

Posted on Originally published at kamenivanov.substack.com

Testing the Service Layer - Part 2: Where the Shared Ancestor Ends (Chapter 10)

#java #testing #qa #architecture

Part 1 of this chapter was about everything AbstractCrudServiceTestCase gets to say once, for every CRUD service that will ever extend it - ownership stamping, idempotent delete, the authorization path shared by update(), loadById(), and delete() alike. That suite works precisely because the behavior it covers really is shared: the same rule, expressed once, correctly inherited by anything that extends the base class. But not everything on ProductsServiceImpl and CategoriesServiceImpl fits that description, and the clearest example is a method the abstract suite never touches at all.

Twins that aren't ancestors

changeStatus() exists independently on both concrete services, and today, the two implementations are nearly identical:

@Transactional
@Override
public void changeStatus(UUID id, ProductStatus newStatus, UUID requesterId) {
    if (requesterId == null) {
        throw new AuthorizationException(UNAUTHORIZED);
    }

    final var product = loadOrThrowNotFound(() -> getDao().loadById(id));
    authorize(product, requesterId);

    if (product.getStatus() == newStatus) {
        return;
    }

    product.transitionTo(newStatus);
    product.setUpdatedById(requesterId);

    getDao().update(product);
}
Enter fullscreen mode Exit fullscreen mode

A same-status request deserved its own explicit decision rather than being left to chance. ProductStatus.ACTIVE.canTransitionTo(ACTIVE) returns false, so without the guard above, requesting a transition to the status a product already holds would throw IllegalStateException. A client retrying a timed-out request would see a failure for a change that had already succeeded. The check mirrors delete()'s idempotency rather than inventing a new definition of it. If nothing would actually change, nothing gets written, not even the audit fields. A retry that touched updatedById on a no-op would leave the audit trail implying movement that never happened. The authorization check still runs before the status comparison. A non-owner requesting the current status doesn't get a silent pass just because nothing would change.

It would be tempting to read that duplication as an oversight. Surely this belongs on AbstractCrudService, right next to create() and update(). It doesn't, for two separate reasons, and both of them are more instructive than the duplication itself.

The first is structural: not every domain in this system has a status at all. AbstractCrudService is generic over any Domain extends AbstractAuditable<UUID>, and forcing a status concept onto that hierarchy would mean either a generic hook that adds ceremony to every future service that never needs status, for example an Order that has no status field to speak of, or a leaky assumption baked into a base class that should know nothing about what any particular domain represents. AbstractAuditable knows about identity and ownership, because every domain object in this system has those. It doesn't know about status, because most won't.

The second reason is the one that actually matters going forward, and it's about where these two methods are headed, not where they are today. Product and Category don't share transition rules: ProductStatus allows a product to move from ACTIVE to OUT_OF_STOCK and back, but CategoryStatus has no equivalent at all. And the side effects each changeStatus() will eventually need to trigger are going to diverge in a way that has nothing to do with today's identical-looking code. When this system integrates with Kafka for event broadcasting, publishing a product to ACTIVE will very likely need to fire an event - a storefront needs to know a product became purchasable. Archiving a category may need to fire an event too, but only on that specific transition, not on every status change a category goes through. Those are two different rules about two different triggers, and they belong on two different methods, sitting inside two different classes, exactly where they already are.

If that behavior had been forced into a shared base method ahead of time - a generic onStatusChanged() hook, parameterized somehow to cover both cases, the divergence would have had nowhere clean to go. You'd end up either parameterizing the hook into meaninglessness, threading flags and conditionals through a method that was supposed to be simple, or growing base-class conditionals that only apply to some subclasses, which is the exact kind of rot this whole series argues against. Two classes having identical code today is evidence of nothing on its own. The real question is whether they're identical because they're expressing the same concept, or identical because they haven't yet diverged. authorize() is the same rule in both classes and will very likely stay the same rule, because ownership-based authorization isn't domain-specific - it's genuinely shared ancestry. changeStatus() is the same code in both classes today for the far less interesting reason that neither one has grown its side effects yet. Promoting the first into the base class was correct. Promoting the second would have been premature abstraction wearing the same clothes as legitimate reuse and premature abstraction is arguably the worse failure of the two, because duplication is at least honest about not knowing yet, while a wrong abstraction actively resists the divergence when it finally shows up.

The test suite reflects that boundary. Each concrete class carries its own full set of changeStatus() tests - null requester, missing entity, wrong owner, valid transition, illegal transition and none of that coverage comes from the abstract suite, because none of it can:

@Test
void changeStatus_WhenTransitionIsIllegal_ShouldThrowAndNeverPersist() {
    final Product existing = persistedProduct(UUID.randomUUID(), requesterId, ProductStatus.ARCHIVED);
    when(productDao.loadById(existing.getId())).thenReturn(existing);
    assertThrows(IllegalStateException.class, () -> productsService.changeStatus(existing.getId(), ProductStatus.ACTIVE, requesterId));
    verify(productDao, never()).update(any());
}
Enter fullscreen mode Exit fullscreen mode

That's not a gap in the abstraction. It's the cost of the abstraction being drawn in the right place. "Extend, don't rewrite" only works as a principle if you're also willing to say, clearly, which things were never meant to be extended from a shared ancestor in the first place. Worth naming directly, while we're on changeStatus(): nothing in this suite would have caught it missing @Transactional. Both implementations follow the same read-modify-write shape as update() - load, mutate, persist and update() carries that annotation for exactly the reason discussed in Chapter 9: it's declarative metadata, not a framework leak, and it matters more once a side effect like Kafka event publishing eventually sits inside the same method. A mocked-DAO test verifies what a method does which calls happen, in what order, under what conditions, but the transactional boundary around those calls is applied by a Spring AOP proxy that never exists in this test setup at all. Catching a missing or misplaced @Transactional requires an integration test with a real Spring context, which is precisely the layer this chapter chose not to test. That's not a flaw in the mocked-DAO approach - it's a reminder that "100% service-layer coverage" from unit tests alone was never actually 100% of what could go wrong.

A specification hiding in a branch

Coverage tooling can tell you that a line or branch executed. It cannot tell you whether the test data and assertions proved the behavior that branch exists to protect.

if (product.getSpecification() == null) {
    product.setSpecification(new ProductSpecification(dto.getDimensions(), dto.getWeight()));
} else {
    product.getSpecification().setDimensions(dto.getDimensions());
    product.getSpecification().setWeight(dto.getWeight());
}
Enter fullscreen mode Exit fullscreen mode

Before this test was added, every update test built an existing product without a specification. The update path therefore always took the if branch and created a new ProductSpecification. The else branch - the normal path for a product that already has a specification was never exercised with a fixture that could prove its intended behavior: mutating the existing object in place rather than replacing it with an equal-looking new one.

The gap was easy to miss because the suite still had broad coverage around updates, and the transformer appeared to work for the creation case. But a flipped condition, a replacement where mutation was intended, or a broken in-place update could all survive until production. The missing test was not merely about increasing a coverage percentage, It had to make the identity of the existing ProductSpecification part of the contract. The fix needed the domain to carry meaningful dimensions and weight, plus a test that could distinguish “the transformer mutated the existing object” from “the transformer built an equal-looking new one.” assertEquals cannot make that distinction on its own:

@Test
void update_WhenSpecificationAlreadyExists_ShouldMutateInPlaceRatherThanReplace() {
    final Product existing = persistedProduct(UUID.randomUUID(), requesterId, ProductStatus.DRAFT);
    existing.setSpecification(new ProductSpecification("10x10x10cm", 450));
    final ProductSpecification originalSpecification = existing.getSpecification();

    final UpdateProduct updateDto = createUpdateDomain();

    when(productDao.loadById(existing.getId())).thenReturn(existing);
    when(productDao.update(existing)).thenReturn(existing);

    final Product result = productsService.update(existing.getId(), updateDto, requesterId);

    // The transformer's else-branch mutates the existing specification rather than
    // allocating a new one - this asserts that behavior directly, not just its effect.
    assertSame(originalSpecification, result.getSpecification());
    assertEquals(updateDto.getDimensions(), result.getSpecification().getDimensions());
    assertEquals(updateDto.getWeight(), result.getSpecification().getWeight());
}
Enter fullscreen mode Exit fullscreen mode

assertSame is doing real work in that test, not just being pedantic. It's the only way to actually prove the else branch ran and mutated the existing object, rather than the if branch running and happening to produce a new object that merely looks equal. Those are two different code paths, guarding against two different bugs, and a weaker assertion would have let either one hide behind the other.

When a fixture stops earning its keep

The suite went through one more revision worth describing, not because the bug it fixed was dramatic, but because it happened inside the test code itself, and the same discipline that applies to production code turned out to apply there too. The original fixture for building an already-persisted entity - needed by every update, delete, and loadById test that requires an existing owner routed through the service's own create() method:

private Domain createPersistedEntity(UUID ownerId) {
    final UUID entityId = UUID.randomUUID();

    when(getMockDao().create(any())).thenAnswer(invocation -> withId(invocation.getArgument(0), entityId));

    final Domain persisted = getService().create(createValidNewDomain(), ownerId);

    assertNotNull(persisted);
    assertEquals(entityId, persisted.getId());
    assertEquals(ownerId, persisted.getCreatedById());

    clearInvocations(getMockDao());

    return persisted;
}
Enter fullscreen mode Exit fullscreen mode

It worked, but it created a dependency that had no business existing: every update, delete, and loadById test in the suite now implicitly depended on create() behaving correctly, purely to get a fixture into existence. If create() ever broke - a transformer throwing, an authorization check misfiring, every ownership test across every concrete service would fail alongside it, for a reason that had nothing to do with what those tests were actually named after. That's a diagnostic cost paid every time someone opens a build with ten failing tests and has to work backward to discover that only one of them represents a real bug. Worse, it needed clearInvocations() afterward, purely to erase the trace that fixture setup had left on the mock, a workaround whose only purpose was to hide the coupling it had just introduced. The replacement drops the indirection entirely, each concrete class already had, separately, a direct helper that builds a persisted entity through its own constructor - no mock, no service call, no cleanup required:

protected abstract Domain buildPersistedEntity(UUID id, UUID ownerId);

private Domain persistedEntity(UUID ownerId) {
    return buildPersistedEntity(UUID.randomUUID(), ownerId);
}
Enter fullscreen mode Exit fullscreen mode 
@Override
protected Product buildPersistedEntity(UUID id, UUID ownerId) {
    return persistedProduct(id, ownerId, ProductStatus.DRAFT);
}

private Product persistedProduct(UUID id, UUID ownerId, ProductStatus status) {
    final var product = new Product(id, Instant.now(), Instant.now(), status);
    product.setName("Existing Product");
    product.setSku("EXISTING-SKU");
    product.setPrice(BigDecimal.TEN);
    product.setCreatedById(ownerId);
    product.setUpdatedById(ownerId);
    return product;
}
Enter fullscreen mode Exit fullscreen mode

Now withId() has exactly one caller left - the create-ownership test, the one place that genuinely needs to simulate what a real DAO does on persistence and buildPersistedEntity() has exactly one job, building a fixture, with zero coupling to whether create() happens to work on a given day. Two mechanisms had grown up to solve what was really one problem, and the fix wasn't clever, just a refusal to let that stand. It's worth naming directly: the same principle this whole chapter argues for in production code - extend or override, don't quietly duplicate a slightly different version of the same mechanism applies just as much to the tests themselves. A test suite that hasn't had this kind of pass run over it tends to accumulate exactly this shape of redundancy, because nobody sets out to write two fixture mechanisms, they arrive one at a time, each locally reasonable, and only look wrong once they're standing next to each other.

What this buys, concretely

Put the whole suite next to the two concrete classes and the claim this chapter opened with is checkable rather than rhetorical. AbstractCrudServiceTestCase carries every test for create(), update(), loadById(), and delete() - authorization guards, not-found guards, ownership stamping, idempotent delete - written once, against the shape every future CRUD service will share. ProductsServiceTestCase and CategoriesServiceTestCase add only what's actually theirs: field mapping through their own transformer, the specification branch in Product's case, and a full changeStatus() suite each, because that behavior was never ancestral to begin with. A future OrdersServiceImpl extending AbstractCrudService inherits the first category for free, the same way ProductsServiceImpl and CategoriesServiceImpl did, and only has to write tests for whatever is genuinely its own.

That's the return on the investment this chapter spent both of its halves on: not a percentage on a coverage report, but a shrinking marginal cost for every service this codebase adds from here forward, a boundary drawn deliberately between what gets inherited and what gets grown independently, and a test suite that fails for the reason its name says it will fail, and nothing else.

What's Next?

The service layer this chapter tested stays deliberately ignorant of anything outside it - no Spring context, no HTTP, no idea that a REST controller exists. That ignorance has a cost that hasn't been paid yet: when AuthorizationException or NotFoundException crosses out of the business logic module, something still has to decide it becomes a 401 or a 404. Chapter 11 is about where that decision lives, and why the domain layer should never be the one making it.

Codebase & Architecture Blueprint

The entire evolutionary architecture of this project is tracked using strict Git tags. To clone the repository and switch exactly to the state established in Chapter 10, use the following link:

Note: All core modules are configured with strict compilation-level boundaries. Compile and run mvn clean install to see the structure in action. Maven version 3.9.* and Java 25 are required.

◀️ Read Chapter 10, Part 1: What the Generic Suite Owes Every Service
▶️ Read Chapter 11: Error Handling and Domain Exceptions Across Module Boundaries (Coming soon)

Join the Masterclass Journey

This article is part of an ongoing weekly series. If you want to receive every upcoming chapter directly in your inbox, alongside deep-dives, infrastructure architecture diagrams, and premium insights before anyone else, subscribe to my Substack Newsletter here!

Recommended Resources for Java & Spring Boot Engineers

Disclosure: I earn a commission if you purchase through the links below, at no additional cost to you.

If you are preparing for Senior/Lead Java interviews or looking to solidify your Spring Boot & Architecture skills, check out these highly-rated resources from the Javarevisited publication (Use promo code friends20 for an exclusive 20% discount automatically applied at checkout):

Top comments (0)