refactor: align app service resolution with Nextcloud container changes - #8407
Merged
Merged
Conversation
vitormattos
force-pushed
the
refactor/nextcloud-app-container
branch
from
September 15, 2026 16:00
765af06 to
a02eff3
Compare
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
vitormattos
force-pushed
the
refactor/nextcloud-app-container
branch
from
September 15, 2026 17:26
c220863 to
4501a33
Compare
vitormattos
marked this pull request as ready for review
September 15, 2026 17:26
35 tasks
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.
Nextcloud master changed how
OCA\...services are resolved between the server and application containers in nextcloud/server#64172.This exposed a LibreSign unit test that registered an app service in the global server container. The same failure was reproduced on LibreSign
mainwith the current Nextcloudmaster, so it is independent of the Observer PR.The upstream change does not remove
OCP\Server::get(): app services requested through it are now resolved from the app container first. The important incompatibility on our side was tests registeringOCA\...services in the global container. During the audit we also found static dependencies that can use normal dependency injection and are easier to test that way.This PR therefore aligns LibreSign with the current container model without replacing service lookups mechanically.
Changes in this PR:
FileServiceinto the SabreSignatureStatusPlugininstead of resolving it through the global service locator;CertificateEngineFactoryTest, where handlers are already constructor-injected;ISecureRandomintoCaIdentifierServiceinstead of resolving a static core dependency at runtime, making its unit test deterministic;CertificateEngineFactoryintoPkcs7Handler, so the production handler does not depend on the fallback service locator inSignEngineHandler;IManagerintoActivitySettingsStore; only the optionalOCA\Activity\UserSettingslookup stays dynamic because that implementation belongs to another app and may not be available.Audit of remaining
Server::get()usage:Pkcs12Handler,IdentifyMethodServiceandAbstractIdentifyMethodare selected by runtime configuration/name. They continue to resolve correctly through the app container with Nextcloud master; replacing them requires a dedicated factory/registry design rather than constructor-injecting a fixed implementation;CrlRevocationCheckerand the root-certificate revocation check inAEngineHandleruse lazy resolution to break the existingCrlService -> CertificateEngineFactory -> engine -> CrlRevocationChecker/CrlServicedependency cycle. Replacing these lookups without first splitting that dependency would introduce a circular constructor dependency;The goal is to keep LibreSign aligned with Nextcloud master and improve testability where dependencies are static, while keeping dynamic/lifecycle-specific resolution only where there is a concrete reason for it.
Related upstream change: nextcloud/server#64172
This PR remains draft until the full PHPUnit matrix against the current Nextcloud branches, including
master, is green.