Skip to content

refactor: align app service resolution with Nextcloud container changes - #8407

Merged
vitormattos merged 11 commits into
mainfrom
refactor/nextcloud-app-container
Sep 15, 2026
Merged

vitormattos merged 11 commits into
mainfrom
refactor/nextcloud-app-container

Conversation

@vitormattos

@vitormattos vitormattos commented Sep 15, 2026

Copy link
Copy Markdown
Member

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 main with the current Nextcloud master, 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 registering OCA\... 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:

  • inject FileService into the Sabre SignatureStatusPlugin instead of resolving it through the global service locator;
  • instantiate the plugin directly with its dependency in the unit test, fixing the failure reproduced with Nextcloud master;
  • remove the obsolete global app-service registration from CertificateEngineFactoryTest, where handlers are already constructor-injected;
  • inject ISecureRandom into CaIdentifierService instead of resolving a static core dependency at runtime, making its unit test deterministic;
  • inject CertificateEngineFactory into Pkcs7Handler, so the production handler does not depend on the fallback service locator in SignEngineHandler;
  • inject the Activity IManager into ActivitySettingsStore; only the optional OCA\Activity\UserSettings lookup stays dynamic because that implementation belongs to another app and may not be available.

Audit of remaining Server::get() usage:

  • dynamic LibreSign implementations in Pkcs12Handler, IdentifyMethodService and AbstractIdentifyMethod are 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;
  • CrlRevocationChecker and the root-certificate revocation check in AEngineHandler use lazy resolution to break the existing CrlService -> CertificateEngineFactory -> engine -> CrlRevocationChecker/CrlService dependency cycle. Replacing these lookups without first splitting that dependency would introduce a circular constructor dependency;
  • migration code and test/bootstrap code that resolves Nextcloud core services is outside the app-service resolution problem exposed by perf: Speed up fetching services from container nextcloud/server#64172;
  • the optional Activity app implementation remains a guarded dynamic lookup because LibreSign cannot require that class to exist.

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.

@github-project-automation github-project-automation Bot moved this to 0. Backlog in Roadmap Sep 15, 2026
@vitormattos
vitormattos force-pushed the refactor/nextcloud-app-container branch from 765af06 to a02eff3 Compare September 15, 2026 16:00
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
vitormattos force-pushed the refactor/nextcloud-app-container branch from c220863 to 4501a33 Compare September 15, 2026 17:26
@vitormattos
vitormattos marked this pull request as ready for review September 15, 2026 17:26
@vitormattos
vitormattos requested a review from a team as a code owner September 15, 2026 17:26
@vitormattos
vitormattos merged commit bafde1f into main Sep 15, 2026
35 of 36 checks passed
@vitormattos
vitormattos deleted the refactor/nextcloud-app-container branch September 15, 2026 17:26
@github-project-automation github-project-automation Bot moved this from 0. Backlog to 4. to release in Roadmap Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 4. to release

Development

Successfully merging this pull request may close these issues.

1 participant