Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.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: Field instance needs uri() method different from the default => 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
Comment #1
dawehnerI just got confused by the title :)
Comment #2
vijaycs85Updating title as per #2057227-9: Field instance needs uri() method different from the default
Comment #2.0
vijaycs85Updated issue summary.
Comment #2.1
vijaycs85Updated issue summary.