Problem/Motivation

We got few static method calls in fieldInstance class

\Drupal::service('plugin.manager.entity.field.field_type')
\Drupal::moduleHandler()
\Drupal::state()

Proposed solution

Inject them.

Reason #

18:22 vijaycs85: is there anyway to inject stuffs in class that implementing FieldInstanceInterface
18:23 vijaycs85: more specifically for https://drupal.org/node/2057227#comment-7725595 /cc timplunkett dawehner
18:23 Druplicon: https://drupal.org/node/2057227 => Field instance needs uri() method different from the default [#2057227] => Drupal core, field_ui.module, normal, needs work, 8 comments, 6 IRC mentions
18:23 dawehner: vijaycs85: that is an interface, right?
18:24 dawehner: vijaycs85: ups i mean an entity
18:24 vijaycs85: dawehner: yeah,
18:24 vijaycs85: dawehner: as I tried to set it in __construct, doesn't look like proper injection as we do in controllerInterface
18:25 dawehner: vijaycs85: so yeah it is an entity so there is no injection available
18:25 dawehner: vijaycs85: tbh. it feels wrong to even think about it
18:26 vijaycs85: dawehner: think about "injection in entity" ?
18:27 vijaycs85: dawehner: if it is wrong, do we have any better way as compare to current? because we are doing Drupal:: in lots of places inside a class. Or do you think it is how it should be?
18:28 dawehner: vijaycs85: the problem is that entity is kind of an data object
18:28 dawehner: vijaycs85: just think of a node
18:28 dawehner: vijaycs85: why should such a data object need a service to work
18:29 dawehner: vijaycs85: sure it in realitity it is different (also see the static methods on entities) but mh, it seems okay to call Drupal:: for now, or use your non-injection thing
18:30 vijaycs85: dawehner: okay... thanks dawehner
18:30 vijaycs85: dawehner++
18:31 katbailey: vijaycs85: dawehner: at the very least you should add a setter and only call Drupal::entityManager() if it is not explicitly set, otherwise this is not unit testable
18:31 vijaycs85: ahh okay..
18:32 vijaycs85: dawehner: or just make as property in __construct
18:32 timplunkett likes wrapping Drupal:: calls in getters when not injectable
18:33 dawehner: katbailey: that is a good idea
18:34 dawehner: katbailey: it works tontally fine to create a mocked container and set it to Drupal::
18:34 dawehner: but I see your point
18:34 katbailey: dawehner: sure it works, but we shouldn't have to do that kind of crap in our tests
19:07 vijaycs85: katbailey++
19:07 vijaycs85: timplunkett++

So provide setter to get service stuff, instead of calling multiple times.

Comments

dawehner’s picture

Title:Inject all container service in FieldInstance class» Inject dependencies in FieldInstance class

I just got confused by the title :)

vijaycs85’s picture

Title:Inject dependencies in FieldInstance class» Provider getter for dependencies in FieldInstance class
vijaycs85’s picture

Issue summary:View changes

Updated issue summary.

vijaycs85’s picture

Issue summary:View changes

Updated issue summary.