LibreSign / LibreSign/libresign
Refactor AccountService to reduce responsibilities and dependencies
@vitorparras is already working on this.
Since Sep 14, 2026.
- Dominant language
- PHP
- Stars
- 818
- Forks
- 146
- Avg merge
- 11h 31m
- Merged PRs (30d)
- 326
Description
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
-
AccountServicehas 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.