1

I have an abstract class called AbstractMediaService and a some specific implementations of this abstract class:

abstract class AbstractMediaService
{
    private $em;
    private $media;
    public function __construct(EntityManagerInterface $em, Media $media)
    {
        $this->em = $em;
        $this->media = $media;
    }

    public function dosomethingInCommon();
    abstract public function dosomethingSpecific();
}

class PhotoMediaService extends AbstractMediaService
{
    public function dosomethingSpecific()
    {
        echo 'i am a photo service';
    }
}

class VideoMediaService extends AbstractMedia
{
    public function dosomethingSpecific()
    {
        echo 'i am a video service';
    }
}

These objects require a Media entity to work with

namespace App\Entity;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Mapping as ORM;
use Doctrine\Common\Collections\Collection;
use Doctrine\Common\Collections\Criteria;

class Media
{}

Controller

/**
 * @Route("/{_locale}/infos/{idMedia}.html", name="info", methods={"GET"}, requirements={
 *   "idMedia" = "\d+",
 * })
 */
public function infosPhotoAction(RequestStack $requestStack, Media $media)
{
   $request = $requestStack->getCurrentRequest();
   $session = $requestStack->getSession();

   $media = new PhotoMedia($media);

   // return response
}

Problem is that I need some dependencies like the Security service or the EntityManager.

I would like to know how autowire AbstractMediaService service.

yivi
  • 42,438
  • 18
  • 116
  • 138
akio
  • 851
  • 9
  • 29
  • 1
    Personally I would just add a MediaServiceFactory class. Let it take care of deciding which service to use and injecting any needed dependencies. – Cerad Apr 07 '22 at 13:35
  • Good idea @Cerad. I focused into the most obvious confusion (attempting to auto-wire an entity). Also, since it's not obvious how `Media` are discerned as being "photo media" and "video media" with the provided code, it seemed beyond the scope of the question. But I'll add that approach to the question (even if I have to make up the "photo"/"video" distinction). – yivi Apr 07 '22 at 13:41
  • Even then, @Cerad, I don't think you could inject the `Media` object at service container level without doing a lot of juggling. The entity is injected in the controller, but would not be directly available the service factory. Yeah, one could inject the `RequestStack`into the factory and attempt to fetch the id from there, etc... but what would be point exactly? Once you are there, a "media" service locator starts looking like a better idea. – yivi Apr 07 '22 at 13:45
  • the distinction is made by the media entity, which have a field in it that defines the type of media as 1 - photo, 2 - video and so on – akio Apr 07 '22 at 13:50
  • What have you tried so far? Where are you stuck? Why not inject these services into the action, just like you injected the `RequestStack` service? – Nico Haase Apr 07 '22 at 13:50
  • In fact I'm not really stuck, I just want do something cleaner as possible. As mentionned by @Cerad it's a good approach to create a factory that will take a mediaentity to determine wich service to create thant use it to work with ? – akio Apr 07 '22 at 13:54
  • 1
    Akio, but the approach mentioned by Cerad wouldn't be enough. What do you understand of that approach exactly? And your question is not "how to do X in the cleanest way possible" (which would be off-topic), but "how to do X", where X does not immediately make sense. – yivi Apr 07 '22 at 13:56
  • you're totally right and it's probably why question was closed by mistake. I understand that this approach could be more flexible – akio Apr 07 '22 at 13:58
  • Quite a discussion. Since your MediaService depends on an entity then you can't wire them with the Symfony service container. But you can wire a factory which in turn uses good old fashion `new` to create your individual MediaService objects. However, there are other more serious issues with your design that I suspect you will soon encounter. – Cerad Apr 07 '22 at 14:53
  • Really ? I’m listening if it could helps me to save my time – akio Apr 07 '22 at 18:38
  • Purpose of these services is to display media and complete the media entity with some extra data with custom queries from repository. If I use entity mapping I have too much database query – akio Apr 07 '22 at 18:42
  • As a general rule, custom queries belong in a repository. The biggest red flag that I see is the use of a generic `Service` suffix. Services should do something and the name should reflect what they do. Names like `Service` and `Manager` implies an unfocused class. And your description does not show why simply passing the entity as an argument would not work. – Cerad Apr 08 '22 at 13:05
  • I use these services to call repository and performs custom queries in order to avoid call repository from controller and do other operations. Should I create 2 services ? One for call custom queries and another to other stuff ? – akio Apr 08 '22 at 17:44

1 Answers1

2

This is wrong. You cannot autowire Media to be injected into a service, because entities are not services.

public function __construct(EntityManagerInterface $em, Media $media)

If VideoMediaService and PhotoMediaService (I renamed them for clarity, since sharing the name with your entity made it look like it were related) need an instance of Media to perform some work, just make that a parameter for the corresponding methods.

public function dosomethingInCommon(Media $media);
abstract public function dosomethingSpecific(Media $media);

Or alternatively, simply have a setMedia(Media $media) method on that class for that:

public function setMedia(Media $media) {
    $this->media = $media;
}

Frankly, this latter approach does not seem like a great idea. You would need to make the methods that work on $media aware of the possibility of setMedia() not having been called yet, or subsequent calls to setMedia() would change how the service behaved. Just making it a parameter of the appropriate method is much cleaner, clearer and safer.

Injecting those services is done like any other service. That they extend an abstract class is irrelevant.

/**
 * @Route("/{_locale}/infos/{idMedia}.html", name="info", methods= {"GET"}, requirements={
 *   "idMedia" = "\d+",
 * })
 */
public function infosPhotoAction(RequestStack $requestStack, Media $media, PhotoMediaService $photoMediaService): Response
{
    $request = $requestStack->getCurrentRequest();
    $session = $requestStack->getSession();

    $photoMediaService->doSomethingSpecific($media)
     
    return new Response('all done');
}
yivi
  • 42,438
  • 18
  • 116
  • 138
  • I was wondering if there was an cleaner approach than that. Is cleaner way to use a factory who takes e Media entity as argument to create the good one service associated ? – akio Apr 07 '22 at 13:56
  • Not necessarily. That was beyond the scope of the question, since the question was not "how to inject one method or another depending on X constraint". For that, I think the cleanest way would be to inject a service locator. You've already asked questions about that. – yivi Apr 07 '22 at 13:57
  • The above approach, for the given constraints, is very clean. Every method declare what it needs. Simple to write, simple to test. – yivi Apr 07 '22 at 13:58
  • but all of you point a very interesting point, I inject PhotoMediaService into controller, but actually I do not know yet what type of media is passed with query parameter in route. – akio Apr 07 '22 at 14:01
  • Great. You could do the same here. Inject all the different MediaServices through a service locator. Which does not change the fact that you wouldn't be injecting the `Media` entity (again, an entity is not a service), but simply _passing_ the entity as an argument to method call. As the question is asked this is the answer to the question. There are other ramifications for you to explore, but that go beyond the scope of this particular question. – yivi Apr 07 '22 at 14:01
  • For the extra scope, check this one: [How to use AutoWiring when looping through Subclasses?](https://stackoverflow.com/a/59575184/1426539) – yivi Apr 07 '22 at 14:03
  • by using service locator, the abstract class AbstractMedia will not become useless ? – akio Apr 07 '22 at 14:04
  • okay, let me be clearer, by using a service locator, should I keep using an AbstractMedia class or it's unnecessary ? – akio Apr 07 '22 at 14:06
  • It's not particularly related. You could continue using it. Or not, and use an interface instead. I guess that if you are using an abstract class is because there is some shared behavior. – yivi Apr 07 '22 at 14:08