Problem/Motivation

Since GitLab CI was enabled (#3628990, #3629003), the phpstan job reports 42 findings. phpstan only gives a warning, so the pipeline does not block, but the findings include two real bugs, deprecated API calls
that break in future versions, and many \Drupal:: calls in classes that already use, or could use, dependency injection.

Real bugs: catches that never run

  • bigbluebutton.install:110: catch (SqlContentEntityStorageException $e) has no use statement, so PHP looks for a class that does not exist and the catch never runs. In the column-adding
    update helper, a field without its own table stops the update instead of being skipped.
  • BigBlueButtonHelper.php:949: catch (Exception $exception) inside namespace Drupal\bigbluebutton means Drupal\bigbluebutton\Exception, which does not exist. When creating
    the BBB webhook fails with a general \Exception, the user gets an error page instead of the "Couldn't create bbb webhook" message.

Deprecated APIs

  • theme_get_setting() is deprecated in Drupal 11.3.0 and removed in 13.0.0.
  • CreateMeetingParameters::addPresentation() is deprecated in bigbluebutton-api-php 3.0 and replaced by addDocument().
  • Feature::LAYOUTS is deprecated: layouts are no longer a valid feature since BBB 4.0.

Dependency injection

37 \Drupal:: calls in classes: 16 in BigBlueButtonHelper, 8 of them for services the class already injects, and 21 in the recording controller, the forms, the recordings formatter, the widget and the
meeting-info REST resource.

Proposed resolution

  • Catches: add use Drupal\Core\Entity\Sql\SqlContentEntityStorageException; to bigbluebutton.install, and change catch (Exception …) to
    catch (\Exception
      …)

    .

  • theme_get_setting(): inject Drupal\Core\Extension\ThemeSettingsProvider as an optional service (@?). On Drupal 10 and 11.0–11.2 the service does not exist,
    and getLogoUrl() falls back to theme_get_setting(). The fallback can be removed when Drupal 10 support is dropped.
  • addPresentation(): use addDocument(new DocumentUrl($file_url)). Without a file name, the XML sent to BBB is byte-identical to addPresentation(), so nothing changes for
    any BBB server version.
  • Feature::LAYOUTS: compare by value ($feature->value === 'layouts'). "Layouts" stays hidden from the disabled-features options, as before.
  • Dependency injection:
    • BigBlueButtonHelper: use the services that are already injected (token, file_url_generator, entity.repository, request_stack), and inject
      cache.default, datetime.time, http_client and date.formatter. The new arguments go after the existing ones.
    • BigBlueButtonRecordingController: inject the helper through create(), and use ControllerBase::config().
    • BBBDeleteRecordingForm: use FormBase::config().
    • BBBLinkForm: bigbluebutton.dynamic_form_factory now passes the helper and the entity type manager into the form. Remove the unused $user = \Drupal::currentUser().
    • SettingsForm: create() through parent::create(), so it works with the ConfigFormBase constructor of every supported core version.
    • BigBlueButtonRecordingsFormatter: create() with the helper, config factory, current user and form builder.
    • BigBlueButtonDefaultWidget: inject the helper.
    • BigBlueButtonGetMeetingInfo: use the helper that is already injected.

No behaviour changes are intended. Nothing sent to the BBB server changes.

Remaining tasks

  • Commit, and confirm that the phpstan job reports no findings for src/ and bigbluebutton.install, and that phpunit is green on Drupal 10 and 11.
  • Follow-up (optional): make the phpstan job blocking once it is clean.
  • Follow-up (optional): offer "layouts" as a disabled feature again for BBB servers older than 4.0.

User interface changes

None.

API changes

Constructors gain arguments: BigBlueButtonHelper (5 new arguments at the end, with the last one optional), BBBLinkForm (helper and entity type manager; $settings is now required),
BigBlueButtonDynamicFormFactory, BigBlueButtonRecordingController and BigBlueButtonDefaultWidget. Inside the module these are only created through the container, create() or the
form factory. Code that creates them directly with new, or extends BBBLinkForm, BigBlueButtonRecordingController or BigBlueButtonDefaultWidget and calls their constructor, must
be updated.

Data model changes

None.

Comments

globexplorer created an issue.

  • globexplorer committed d2e9b955 on 1.0.x
    task: #3629005 Fix phpstan findings: dependency injection, deprecated...