Refactor AccountService to reduce responsibilities and dependencies

[vitormattos]

Context

While improving the AccountService unit tests in #8142 as part of #8053, we found that some tests need a large amount of setup and many mocks to test one behavior.

This is not mainly a test problem. AccountService currently has many dependencies and handles several different responsibilities.

For example, it handles:

• signer-to-account creation;
• identification method updates;
• new user email;
• account configuration and preferences;
• visible signature element creation, update and deletion;
• loading visible element images from URLs or base64;
• PFX upload and validation;
• certificate password updates, reading, deletion and revocation;
• file lookup for signature requests.

Because these responsibilities are in the same service, some unit tests need to configure dependencies that are not directly related to the behavior being tested.

Goal

Reduce the responsibilities and dependencies of AccountService by moving cohesive behavior to the services where it belongs.

The public behavior should stay compatible. This issue is about improving the internal architecture and testability, not changing the user-facing flow.

Existing services

Before creating new services, check if the responsibility already has a natural place in an existing service.

In particular, SignerElementsService already handles visible signature elements, including user elements, session elements and their files.

The create, update and delete operations that are currently in AccountService should be reviewed to see if they should also belong to SignerElementsService.

Other existing services and handlers should be reviewed in the same way before adding new abstractions.

Possible boundaries

The exact design should be decided during the refactor, but the current responsibilities suggest these areas:

• signer-to-account creation and identification update;
• account configuration and preferences;
• visible signature element management;
• PFX/certificate lifecycle;
• signature-request file lookup.

This does not mean that one new service must be created for every area.

Prefer moving behavior to an existing cohesive service when possible. Create a new service only when there is no good existing owner for that responsibility.

AccountService may remain as a small coordinator when coordination between these responsibilities is needed.

Tests

The refactor should make the tests smaller and more focused as a result of clearer production-code boundaries.

When moving behavior:

• keep or improve the existing business-rule coverage;
• move tests to the matching service when responsibility moves;
• prefer data providers for repeated cases;
• avoid solving production complexity only with large test helpers;
• avoid tests that need unrelated mocks;
• run Infection for the affected source/test pairs.

The goal is not only to reduce the number of lines in AccountServiceTest. The production code should have clearer responsibilities and fewer unrelated dependencies.

UUID cache behavior

The tests added in #8142 also exposed an important behavior in getSignRequestByUuid() and getFileByUuid().

AccountService stores the current sign request, database file and file node in instance properties. The current cache does not record which UUID the cached data belongs to.

For example, this sequence should be reviewed:

$service->getFileByUuid('uuid-a');
$service->getFileByUuid('uuid-b');

We need to verify that the second call cannot return data that belongs to uuid-a.

Do not preserve the current cache only because a mutation test expects repeated calls to use it.

If this behavior is a bug, fix it or create a focused bug issue if the change should be handled separately from the refactor. If caching is still useful, it must be safe when different UUIDs are used.

Scope

This work can be implemented in more than one PR.

Each PR should move one cohesive responsibility and keep the existing behavior covered by tests.

Avoid rewriting the complete service in one large PR.

Related to #8142 and #8053.

Acceptance criteria

• AccountService has fewer unrelated responsibilities and dependencies.
• Existing services are reused when they are the natural owner of a responsibility.
• New services are introduced only when there is no suitable existing service.
• Extracted responsibilities have clear boundaries.
• Existing public behavior remains compatible.
• Relevant existing tests continue to pass.
• Tests for moved behavior are moved or added to the appropriate test classes.
• Tests do not need unrelated mocks to verify simple behavior.
• The UUID/file cache behavior with different UUIDs is verified.
• Unsafe cache behavior is fixed or tracked separately if needed.
• Infection is checked for the affected source/test pairs.

[vitorparras]

Hi! I’m going to work on this issue. I’ll approach the refactor incrementally, moving one cohesive responsibility at a time to the appropriate existing service where possible, together with the related tests. I’ll also review the UUID cache behavior described in the issue.