Jump to content

Architecture Summit 2014/TitleValue

From mediawiki.org

TItle Value

[edit]

SQL Abstraction was moved to https://etherpad.wikimedia.org/p/sql_abstraction

Daniel Kinzler: https://www.mediawiki.org/wiki/Requests_for_comment/TitleValue (RFC is a proof of concept)

Main goal is improving modularity using dependency injection, etc.

  • improve maintainability
  • testability
  • reusability
  • ramp-up for newcomer contributors

Title used as the prime example of code that would need to be refactored

  • highly interdependent with other mediawiki code
  • Everything depends on it
  • Hard to test
  • Hard to reason about

TitleValue is refactored/extracted version

  • No idea of how to load page
  • No idea how to generate HTML
  • Used by Services which know how to do these things

Proposed diff at: https://gerrit.wikimedia.org/r/#/c/106517/

  • Uses Dependency Injection
  • Results in the ability to write unit tests much more easily using mocks
  • Unit tests provide better coverage with less work than integration tests that are required with current code design

To make links, have various LinkRenderer classes, factoring out stuff in Linker and get rid of the static interface there / make it legacy.

use of TitleValue in Special:Categories:

Goal

[edit]
  • Tim: in principle support for what daniel proposes, for value objects / service classes?

Concerns:

  • Chad: not sold on the idea. Sees concenrs with testability. Thinks the proposal makes things more complex. Title is the kitchen sink of all objects, though. This may make things unintuitive. Each time I see the proposal, I fear that we're making something that's more complex, not convinced that this reduces barriers to entry for coding with MediaWiki.
  • Nik: Elaborations because of Java background. Nik has seen the opposite of the Title case. I.e. don't push this model all the way.
  • Chris: review can be encumbered by a difficult Dependency Injection implementation
  • Gabriel: We need to get the granularity right. Good to start with coarser level first, and see where to go from there. Owen +1s.

Rob: continous spectrum of opinion from Current State ----> Java AbstractFactoryFactoryBuilderHolder in terms of how granular and involved the Dependency Injection implementation is.

  • Room was roughtly divided between pro and on the fence on first straw pool
  • Tim support for the proposal on grounds that the design principle is sound and better than what we have now. Avoid going too far by setting down rules on what constitute needless complexity.
  • Daniel on complexity: two kinds of complexity. Complexity of the network of objects and complexity of the code. This proposal simplifies code complexity and moves the complexity to thinking about the network of objects
  • Dan A.: the nightmare scenario here is just configuring DI through several layers of XML that are just there to augment a poor implementation. Solutions: convention over configuration, etc.
  • Owen: Wikia uses a php extension hack to provide testabilty by monkey-patching `new`. They favor this approach as an alternative. (I wouldn't say "favor", it was forced necessity :)
  • Yuri: Sees this as a step to expanded functionality due to separation of concerns. Also leads to the ability to create a service orientied architecture. +1 from owen
  • Brad: 1 class with 5 functions vs 5 classes (plus parent classes, etc) with 1 function
  • Daniel: role isolation and separation of concerns means that we can refactor the 5 classes into maybe 3 classes because we will find commonality while refactoring.
  • Rob: historical fact is that one of two things would happen from here: 1. proposal reworked until it's merged and 2. proposal festers. The point of the summit is to avoid 2. and Rob asked the room and Tim for guidance.
  • Tim: seeking consensus
  • Roan: people agree that service classes are reasonable but disagree about the granularity. Maybe we should agree on guidelines for when things should be split out or not and a target list of things to refactor
    • Not just for unit tests
    • Actually reusable (and making something reusable implies a long term commitment because others will start using it)
    • "I want to be able to query title existance across wikis" as example use case
  • Daniel: TitleFormatter & LinkGenenerator are the services that primarily need to be discussed
  • Yuri: there is code duplication between API and Core that this could address
  • Roan: We should find the services that a lot of people agree we need to refactor via on-list discussion. Focusing on these popular candidates will allow us to evaluate the approach in general.
  • avoid AbstractFactoryBuilderHolderRegistrySingletonProxyBean

Decision: Daniel will help with the process to identify desired service interfaces but is not sure if he has enough time to drive the process. Looking for people to help drive it.

Nik stepped up in a big way and volunteered to lead the discussion :)

Q&A

[edit]
Owen mentions that Wikia has taken a different approach to making core code testable (php-test-helpers extension overrides the "new" operator so you can return mocks without needing DI)
Pulling up an example: https://github.com/Wikia/app/blob/dev/extensions/wikia/SEOTweaks/tests/SEOTweaksTest.php#L796
Sample line: $this->mockClass( 'Title', $mockTitle );
Can also mock static constructors: $this->mockClass( 'Title', $mockTitle, 'newFromText' );
14:15 done with TitleValue. See https://etherpad.wikimedia.org/p/sql_abstraction for the second session in this block.
Retrieved from "https://www.mediawiki.org/w/index.php?title=Architecture_Summit_2014/TitleValue&oldid=3615641"