Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(REF) Extract compiler passes from CRM_Api4_Services #24283

Merged
merged 2 commits into from
Aug 17, 2022

Conversation

totten
Copy link
Member

@totten totten commented Aug 16, 2022

Overview

CRM_Api4_Services has interesting behavior that (sort of) applies to all container-based services -- it detects services with the tags event_subscriber and spec_provider, and it registers them with the necessary provider (ie the event-dispatcher or the spec-gatherer).

This patch preserves support for these tags, but it moves the code to a more conventional spot, fixes a bug, and adds explanatory docblocks.

Before

It scans for tagged services (findTaggedServiceIds()) during the definition phase. This is too early. The tags will work for some services -- but they'll be quietly ignored on other services.

For example, if you define a tagged-service in Civi\Core\Container::createContainer(), then it will be recongized. But if you define the same tagged-service in hook_civicrm_container, then it won't be recognized.

After

It scans for tagged services (findTaggedServiceIds()) during the compilation process. This is slightly later -- after all service definitions have been defined.

Consequently, the tags will work the same way (regardless of who defines the service).

Technical Details

The key thing is to distinguish these use-cases:

  • You may wish to define a service. This is generally done by calling $container->setDefinition() (et al).
  • You may wish to define a generic handler to enhance other services. This is generally done by calling $container->addCompilerPass()

The prior code conflates these two things. The PR splits them apart (running as separate phases - in keeping with the general design of Symfony's ContainerBuilder).

Comments

The specific folders / interfaces / tags for CRM_Api4_Services may seem a bit alien. I submit that doesn't matter -- because there's a bunch of test-coverage that relies on CRM_Api4_Services(if it breaks, you'll see a lot of failures) and because it's a step toward #24276, which will allow a more consistent model.

totten added 2 commits August 16, 2022 15:21
The `CRM_Api4_Services` implementation has a secret behavior that applies
to all services - it detects the tag 'event_subscriber' and registers with
the dispatcher.

This kind of behavior is often implemented as "compiler pass" and given its
own documentation.
The `CRM_Api4_Services` implementation has a secret behavior that applies
to all services - it detects the tag 'spec_provider' and registers
with the spec-gatherer.

This kind of behavior is often implemented as "compiler pass" and given its
own documentation.
@civibot
Copy link

civibot bot commented Aug 16, 2022

(Standard links)

@colemanw
Copy link
Member

Tests pass, so it works!

@colemanw colemanw merged commit ae3b0f3 into civicrm:master Aug 17, 2022
@totten totten deleted the master-api4-passes branch August 17, 2022 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants