Commit graph

8 commits

Author SHA1 Message Date
DannyS712 568dbebd47 tests: clean up requirements tests
Convert to unit tests, no integration needed
Use data providers

Change-Id: Ife758952b6bf8a046eddf4bdc478437564f0bc22
2021-03-10 14:45:24 +00:00
Sam Smith d4c2e2b441 [Hygiene] featureManager: ComplexRequirement -> Requirement
FeatureManager::registerRequirement registering an instance of
SimpleRequirement with ::registerComplexRequirement was awkward.

Changes:

* Rename FeatureManager::registerRequirement to
  ::registerSimpleRequirement, which is exactly what it does!

* Rename FeatureManager::registerComplexRequirement to
  ::registerRequirement

Bug: T244481
Change-Id: I612af959cee4cdcd0bdcda51a81b86ed61ee2e16
2020-04-29 15:26:50 +01:00
Stephen Niedzielski 6b6bed86ed [docs] [dev] [PHP] [FeatureManager] revise docs and add DynamicConfigRequirement test
Address some feedback from I7a2cdc2dfdf20d78e4548f07cf53994563b234b3:

- Miscellaneous documentation improvements.
- Add a false case test to `DynamicConfigRequirement->isMet()`.

Bug: T244481
Change-Id: Ic5637f42da755f871c5a6d545e14effd3ac8c670
2020-03-26 20:39:40 +00:00
Sam Smith 84226b41b6 featureManager: Add DynamicConfigRequirement requirement
We expect the vast majority of requirements and features to be defined
in services as possible. However, there are some "complex" requirements
that require additional application/HTTP request state. Unfortunately,
service wiring is done before some of that state is available.

I65702426 attempted to work around this by requiring clients of the
Feature Manager to pass that additional state on every interaction with
the system. Those complex requirements would then select the parts of
the state that they required when it was required. However
implementations of \IContextSource are God objects and their use should
be limited.

Whilst reviewing I65702426, Stephen Niedzielski mentioned that the
application state being available is a requirement. This remarkably
simple solution:

- Keeps the Requirement interface and FeatureManager API free of God
  objects;

- Is true to the nature of the Feature Manager - it makes clear and
  centralizes the various checks for application state being available
  across the codebase; and

- Inject a Requirement implementations' dependencies at construction
  time

It just so happens that the $wgFullyInitialised variable flags whether
the application state is available...

Changes:

- Add the the FeatureManager\Requirements\DynamicConfigRequirement class
  and tests. The DynamicConfigRequirement lazily evaluates a single
  configuration value whenever ::isMet is invoked

- Register an DynamicConfigRequirement instance, configured to evaluate
  $wgFullyInitialised while constructing the Vector.FeatureManager
  service

Bug: T244481
Change-Id: I7a2cdc2dfdf20d78e4548f07cf53994563b234b3
2020-03-18 10:10:42 +00:00
Sam Smith d14caf2f11 featureManager: Add Requirement interface
FeatureManager::registerRequirement established the interface for a
requirement: its name and whether it's met.

However, the Feature Manager also needs to handle scenarios where a
requirement needs additional context before it can be considered met.
That context may not be available when the application is booting, e.g.
checking if the user is logged in; or the logic is complicated enough
that it should be under test.

Changes:

- Add the Requirement interface and update FeatureManager to work with
  implementations of it

- Maintain B/C by constructing an instance of a the SimpleRequirement
  DTO

Bug: T244481
Change-Id: Id95d9e5d7125492968d0e15515224aadbc3075f8
2020-03-09 11:06:26 +00:00
Sam Smith 64bd4601b4 featureManager: Add typehints
I735fd640 bumped the required MediaWiki version to 1.31. That version
dropped support for PHP 5.x.

Wherever possible, update FeatureManager's methods to use PHP 7.0.x's
scalar and return type declarations.

Bug: T244481
Change-Id: Ib5636d0ec5ec7f0c93b5b3317a12635668b589e2
2020-03-09 11:05:21 +00:00
Sam Smith 3f95820467 featureManager: Set -> Requirement
As was noted in https://phabricator.wikimedia.org/T244481#5859513, the
term "set" doesn't seem natural. Piotr Miazga (polishdeveloper, pmiazga)
and Nicholas Ray (nray) suggested a number of good replacements,
including "requirement." Serendipitously, this term is already used in
FeatureManager's documentation.

Bug: T244481
Change-Id: I559c2d4149db69235cdd4bb880697deb1a145743
2020-03-06 17:23:46 +00:00
Sam Smith 9177c22365 features: Add minimalist feature manager
With complex additions to Vector's codebase like the Desktop Improvement
Program upcoming, it's important that we have a shared, intuitive
language to talk about features and their requirements. Centralising
the registration of features and creating an API satisfies does exactly
this.

This change introduces a greatly-reduced version of Piotr Miazga's
(polishdeveloper, pmiazga) original proposed API and associated
scaffolding classes for feature management in Vector, which itself was
based upon his work in MobileFrontend/MinervaNeue. This is done to
establish a foundation upon which we can build the more sophisticated
parts of Piotr's proposal in a piecemeal basis, thereby minimising risk.

Distinct from Piotr's proposed API is the ability to register sets and
features that are always enabled or disabled.

Additionally:

- A Vector.FeatureManager service is registered but not used

- A list of proposed immediate next steps is included

Bug: T244481
Change-Id: Ie53c41d479eaf15559d5bb00f269774760360bde
2020-02-26 11:49:29 +00:00