Service to support different types of resources

[NotionCommotion]

I've asked this question before but still haven't really figured out how best to implement it, and have found myself hitting another roadblock.

I have multiple types of charts (i.e. bar, pie, etc), and various endpoints to access them.  I like to keep my routing script uncluttered, and feel the following does so.

$app->get('/chart', function (Request $request, Response $response) {
    //List of chart with optional filtering
    $this->chartService->index($request->getQueryParams());
    return $this->chartResponder->index($response, $index);
});

$app->post('/chart', function (Request $request, Response $response) {
    //Create a new chart of type (i.e. bar, pie, etc) specified by type parameter
    $chart=$this->chartService->create($request->getParsedBody());
    return $this->chartResponder->create($response, $chart);
});

$app->get('/chart/{id:[0-9]+}', function (Request $request, Response $response, $args) {
    //View chart of given ID
    $chart=$this->chartService->detail((int)$args['id']);
    return $this->chartResponder->delete($response, $chart);
});

$app->delete('/chart/{id:[0-9]+}', function (Request $request, Response $response, $args) {
    //Delete chart of given ID
    $this->chartService->delete((int)$args['id']);
    return $this->chartResponder->delete($response, null);
});

$app->post('/chart/{id:[0-9]+}/series', function (Request $request, Response $response, $args) {
    //Add a new series to the collection for chart with given ID
    $chart=$this->chartService->addSeries((int)$args['id'], $request->getParsedBody());
    return $this->chartResponder->update($response, $chart);
});

$app->put('/chart/{id:[0-9]+}/series/{seriesPosition:[0-9]+}', function (Request $request, Response $response, $args) {
    //Modify series of given position in the collection for chart with given ID
    $chart=$this->chartService->updateSeries((int)$args['id'], (int)$args['seriesPosition'], $request->getParsedBody());
    return $this->chartResponder->update($response, $chart);
});

//More endpoints for chart of given ID...

I then created the following service to support the endpoints.  Off topic, but is this a service or a controller?

My index() method is chart type agnostic.  My create() method needs a means to determine what type of chart to create, and does so using the received chart "type" passed in the body to get the applicable repository.  All the other methods receive the chart ID in the URL path and use it to first get the chart entity and then get the applicable repository based on the object.  All seems good!

<?php
namespace NotionCommotion\ChartBuilder\Service;
use NotionCommotion\ChartBuilder\Entity\Chart;

class ChartService
{
    protected $em;
    
    public function __construct(\Doctrine\ORM\EntityManager $em)
    {
        $this->em = $em;
    }

    public function index(array $params=[]):array {
        return $this->em->getRepository(Chart::class)->index($params);
    }

    public function create(array $params):Chart {
        //Get the specific repo based on $params['type].  Not perfect, but good enough
        $discriminatorMap=$this->em->getClassMetadata(Chart::class)->discriminatorMap;
        $repo = $this->em->getRepository($discriminatorMap[$params['type']]);
        //Validate data
        $chart=$repo->create($params);
        $this->em->persist($chart);
        $this->em->flush();
        return $chart;
    }

    public function read(int $id):Chart {
        return $this->em->getRepository(Chart::class)->find($id);
    }

    public function delete(int $id):void {
        $this->em->remove($this->read($id));
        $this->em->flush();
    }

    public function addSeries(int $idPublic, array $id):Chart {
        $chart=$this->read($id);
        //Validate data
        $repo=$this->em->getRepository(get_class($chart));
        $repo->addSeries($chart, $params);
        $this->em->persist($chart);
        $this->em->flush();
        return $chart;
    }

    public function updateSeries(int $id, int $position, array $params):Chart {
        $chart=$this->read($id);
        $series=$chart->getSeries();
        $seriesNode=$series->offsetGet($position);
        //Validate data
        $repo=$this->em->getRepository(get_class($seriesNode));
        $repo->update($seriesNode, $params);
        $this->em->persist($chart);
        $this->em->flush();
        return $chart;
    }
}

Until...  I find myself needing to put non-database related functionality in the repository and violating the single responsibility principle.  I am not a complete purist and might be willing to do so, however, there does not appear to be a clean way to inject dependencies in a Doctrine repository.

One thought I had was to create specialized services maybe as follows:

$c['chartService'] = function ($c) {
    return new ChartService(
        $c[EntityManager::class],
        [
            'bar'=>function ($c) {return new BarChartService($c[EntityManager::class]);},
            'pie'=>function ($c) {return new PieChartService($c[EntityManager::class], $c['someOtherObject']);},
            //add more types...
        ]
    );
};

 

class ChartService
{
    protected $em, $subServices=[];
    
    public function __construct(\Doctrine\ORM\EntityManager $em, $subServices)
    {
        $this->em = $em;
        $this->subServices = $subServices;
    }

    public function index(array $params=[]):array {/* no change */}

    public function create(array $params):Chart {/* maybe no change */}

    public function __call($name, $args) {
        $chart=$this->em->getRepository(Chart::class)->find($args[0]);
        $subservice=$this->getSubservice($chart);
        $args[0]=$chart;
        return $subservice->$name(...$args);
    }
}

abstract class AbstractSpecificChartService
{
    protected $em;
    
    public function __construct(\Doctrine\ORM\EntityManager $em)
    {
        $this->em = $em;
    }

    public function __call($name, $args) {
        throw new \Exception("Method $name not supported");
    }

    protected function getSubservice(Chart $chart):self {
        //Haven't figured out but can do so if needed.
    }

    public function delete(Chart $chart):void {
        //Include methods common to all charts here
        $this->em->remove($chart);
        $this->em->flush();
    }

    protected function helperMethods($foo) {
        //If necessary.
    }
}

class BarChartService extends AbstractSpecificChartService
{
    //Override __construct if necessary

    public function updateSeries(Chart $chart, int $position, array $params):Chart {
        //Note that Chart and not $id is passed.
        //implement code as needed
        return $chart;
    }
}

While this will provide some flexibility, I've been stung more than once using inheritance when injection should be used, and am worried it will lead to no good.  But what would I inject?  I previously suggested injecting a BarChart into a Chart and received a response that doing so is nonsensible.

So, after this long story, how should I implement this?  Thank you

[requinix]

"Service" is probably a more accurate term. It may be functioning like a controller in that it has various actions called for requests, but it isn't directly interacting with the request or response so it's not quite entirely a controller.

You're talking about single-responsibility with the chart creation bit? You're still observing it: the single responsibility of creating charts is in one central location.

Your method seems more or less right: single place to look up chart data, chart identifier maps into a class through a factory, class handles the precise work of the various operations. Chart types are not dynamically created (eg, no user-defined types) so hardcoding identifiers and class names is acceptable.

[NotionCommotion]

Thanks requinix.  I feel I have come a long way thanks to the good advice I have received on this forum.

From your response, I assume I should not attempt to implement my sub-service idea, true?  Doesn't hurt my feelings as I would rather not.

What if I had to do something different for only a specific type of chart.  For instance, I have these remote charts that need to perform a cURL request to another server.  Before making this request, I want to verify everything locally, and if all good, perform the remote update, and then save locally.  I suppose I can do something like the following, but it seems so ugly.  Or I can perform this scope in the specific RemoteChart repository, but I am not sure how to get the applicable objects in the repository other than hard coding $remoteConnection=new RemoteConnection($url, $port, $etc); in the repository.

<?php

class ChartService
{
    public function updateChart(int $id, array $params):Chart {
        $chart=$this->read($id);
        //Validate data
        $repo=$this->em->getRepository(get_class($chart));
        $repo->update($chart, $params);
        if($chart->isType('remote')) {
            try{
                $rsp = $this->remoteConnection()->updateChart($id, $params);
                //Maybe do something with $rsp
            }
            catch (RemoteConnectionException $e) {
                //clean up.
                throw new ChartServiceException($e->getMsg());
            }
        }
        $this->em->persist($chart);
        $this->em->flush();
        return $chart;
    }
}

 

[requinix]

RemoteChart sounds like it's more of a data thing than a chart thing. Sounds like you need to abstract out the data retrieval/storage process. Probably specialized for the way charts will use it, but more or less separate from the actual chart business.

[NotionCommotion]

25 minutes ago, requinix said:

Sounds like you need to abstract out the data retrieval/storage process.

Agree I think, but I am not certain where to start.

My partial implementation I showed for individual services for each chart type was an attempt to do so.  All services will implement an interface with methods updateChart(), deleteChart(), etc, and only the applicable service will perform the data scope so that the initiator (router) doesn't need to be aware of these details.

Or by injecting (or even hard coding) a separate class in the applicable repository, the initiator (service in this case) will not need to be aware of these details.

Is this what you mean by "abstract out"?  I don't, however, think I am going down the right path.  Can you elaborate?

Thanks

[requinix]

Can you decouple the storage mechanism from the chart? Entirely? The general chart service would lookup the basic chart information, determine the data source, and inject that into the chart.

[NotionCommotion]

12 hours ago, requinix said:

Can you decouple the storage mechanism from the chart? Entirely? The general chart service would lookup the basic chart information, determine the data source, and inject that into the chart.

Need the service use some if/then logic to look up the information and determine the data source, or typically is this performed elsewhere?  Inject the data into the chart entity?

Also, my issues are not related to populating a chart with data and displaying that chart, but creating and modifying the metadata which defines those charts such as adding a series, etc.  Actually, I expect you know that.  What you might not know is the remote chart is not just reaching out to some other server to get data, but reaching out to some other server to instruct it to start trending some physical variable (or how to trend it) so that it can be used in a future chart.  I should have named the method something more descriptive.  What I could (and will) do is decouple this from the general chart and put it in maybe the ChartNode where a bar, pie, etc chart can have either a LocalChartNode or a RemoteChartNode.  But the need still remains to determine whether the given chartNode is remote or not and if so perform some cURL query.  And I think I need to push that functionality down and abstracting it from the service, no?  But how to do so is still a question.

if($chart->isType('remote')) {
    try{
        $this->remoteConnection()->tellRemoteServerToTrendPhysicalVariable($id, $params);
    }
    catch (RemoteConnectionException $e) {
        //clean up.
        throw new ChartServiceException($e->getMsg());
    }
}