5
\$\begingroup\$

I have constructed an API service that dives into a storage engine and retrieve data.

I have tried my best to apply the MVC modelling with services (repository) in mind where the controller (as it should) is just the data direction (input goes in and it redirects the output with the Repo in the middle doing all the heavy lifting).

The framework is Silex (the small version of Symfony).

For all purpose in keeping the code simple - I'm only going to paste 1 core controller function rather than everything (as it is not needed to comprehend). If you need more info - do not hesitate in asking.

Currently I have 2 version of the same controller so that I can test the end results (bad practice I am aware, as I am suppose to use an interface to inject the specific factory object in order to use the correct repository... except I'm having a small problem wrapping my head around it so I choose to have 2 controllers).

The first version - is DB driven (using Eloquent from Laravel) - the second version is using ElasticSearch. There will be a third version using a NoSQL (which is why I want to efficiently refactor the MVC to be extensible).

Please keep in mind for any recommendation/answer given that I will be plugging in a future storage engine.

Version 1 DB Driven

class MusicController extends BaseController
{
    const MUSIC_REPOSITORY_CLASS_NAME = '\Api\Manager\Media\Music\MusicRepository';
    const MUSIC_MEDIA_NAME            = 'music_album';
    const MUSIC_MEDIA_ENTITY_NAME     = '\Api\Entity\Music\Album';
/**
 * __constructor to save Application Container.
 *
 * @param Application $app Application Container.
 *
 */
public function __construct(Application $app)
{
    parent::__construct($app);
}

public function getPopularListAction($device, $limit, $offset)
{

    $musicRepo = new MusicRepository();
    $where     = array(
        'mediaTypeId'    => $musicRepo::MEDIA_TYPE_MUSIC_ID,
        'mediaTypeName'  => $musicRepo::MEDIA_TYPE_MUSIC_NAME,
        'device'         => $device,
        'status'         => 'active',
    );

    $albums  = $musicRepo->getByPopularity(
        self::MUSIC_MEDIA_ENTITY_NAME,
        $where,
        $limit,
        $offset
    );
    return $albums;
}
}

class MusicRepository
{

public function getByPopularity($mediaEntityName, $where, $limit, $offset)
{
    $mediaObject = new $mediaEntityName();
    $mediaTableName = $mediaObject->getTable();
    $mediaTypeName = $where['mediaTypeName'];

    $album = $mediaObject
        ->selectRaw('music_album.*, count(music.id) as songs_count, score')
        ->SongsWithInnerJoin()
        ->with('artists')
        ->with('provider')
        ->having('songs_count', '>', '0')
        ->take($limit)
        ->skip($offset)
        ->orderBy('score', 'DESC');

    $returnResult = $album->get()->toArray();

    return $returnResult;
}

}

class Album extends Eloquent
{
/**
 * @var string  $table      music_album
 * @var string  $primaryKey id
 * @var bool    $timestamps no timestamp autosave
 */
protected $table      = "music_album";
protected $primaryKey = 'id';
public $timestamps    = false;

/**
 * Return hasMany relationship with Music Entity ordered by id
 * @return mixed
 */
public function songs()
{
    return $this->hasMany('Api\Entity\Music\Music','album_id','id')->OrderBy('id','ASC');
}

/**
 * Return hasMany relationship with Music Entity ordered by id
 * @return mixed
 */
public function artists()
{
    return $this->belongsToMany(
        'Api\Entity\Music\Artist',
        'music_album_artists',
        'album_id',
        'artist_id'
    );
}
/**
 * Inner Join with music table
 * @param $query    current query
 */
public function scopeSongsWithInnerJoin($query)
{
    $query->join('music as m', 'm.album_id', '=', 'music_album.id');
}
}

VERSION 2 - ElasticSearch Storage Engine

class BaseController
{

protected $app;

public function __construct(Application $app)
{
    $this->app = $app;

}

public static function after($content)
{
    $esOutput = new OutputFormat($content);
    $normalizedContent = $esOutput->formatData();

    $content = array(
        'statusCode' => 200,
        'content'    => $normalizedContent
    );

    return $content;
}
}

class MusicController extends BaseController
{

const MUSIC_REPOSITORY_CLASS_NAME = '\Api\Manager\Media\Music\MusicRepository';
const MEDIA_ELASTIC_REPOSITORY_CLASS_NAME = '\Api\Manager\Media\v2\ElasticMediaRepository';
const MEDIA_REPOSITORY_CLASS_NAME = '\Api\Manager\Media\v2\MediaRepository';
const MUSIC_MEDIA_NAME = 'music_album';
const MUSIC_MEDIA_ENTITY_NAME = '\Api\Entity\Music\Album';

/**
 * Fetch Music Album sorted by Popularity
 *
 * @param $device   device type id
 * @param $limit    max number of results returned
 * @param $offset   starting index of result
 *
 * @return mixed
 */
public function getPopularListAction($device, $limit, $offset)
{

    $mediaSearchRepo = new ElasticRepository();

    $where = array(
        'mediaTypeId'   => MUSIC_REPOSITORY_CLASS_NAME::MEDIA_TYPE_MUSIC_ID,
        'mediaTypeName' => MUSIC_REPOSITORY_CLASS_NAME::MEDIA_TYPE_ALBUM_NAME,
        'device'        => $device,
        'status'        => 'active',
    );
    $result = $mediaSearchRepo->getPopularList($where, $limit, $offset);

    return $result;

}
}

class ElasticRepository
{

public function getPopularList($where, $limit, $offset)
{
    $config = \Lib\Config::get('elasticsearch');
    $es = new \Elasticsearch\Client(array('hosts'=>array($config['server'])));

    $mediaType = $where['mediaTypeName'];
    $extraFilter = '';
    if ($mediaType !== null) {
        $extraFilter = '
              ,{
                "term": {
                        "media_type": {
                            "value": "'.$mediaType.'"
                        }
                    }
              }
            ';
    }
    $json = '{
                    "query": {
                     "filter": {
                        "bool": {
                           "must": [
                              {
                                 "term": {
                                    "status": {
                                       "value": "active"
                                    }
                                 }
                              }
                              '.$extraFilter.'
                           ]
                        }
                     }
                  }
               },
               "sort": [
                  {
                      "_score":{
                        "order": "desc"
                      },
                     "score": {
                        "order": "desc"
                     }
                  }
               ],
                "from":'.$offset.',
                "size":'.$limit.'
            }
        ';
    $params['index'] = $config['index'];
    $params['type'] = $config['type'];
    $params['body'] = $json;
    $result = $es->search($params);
    if (isset($result['hits']['hits']['0']) && isset($config['minRelevanceScoreToReturn'])) {
        if ($result['hits']['hits']['0']['_score'] < $config['minRelevanceScoreToReturn']) {
            return false;
        }
    }
    return $result['hits']['hits'];
}
}

class OutputFormat
{
protected $originalData = null;
protected $outputData = null;


public function __construct($data = array())
{
    $this->originalData = $data;
}

public function formatData()
{
    if ($this->originalData == null) {
        return $this->outputData;
    }
    $this->outputData = $this->originalData;

    if(is_array($this->originalData)) {
        if (isset($this->originalData[0]->_source)) {
            $this->outputData = array();
            foreach($this->originalData as $data) {
                $this->outputData[] = $data->_source;
            }
        }
    }

    return $this->outputData;
}
}

Since this is an API - Ideally I would like it to be returned in an array format so that in the "view" it is shown as a json.

Notes: Version 1 - its Eloquent (from laravel) driven - the object is stemming from the DB's fields - So to me when creating Album makes sense to extend it from Eloquent to retrieve and let the Repository do the querying (and not the controller).

However clearly as shown in version 2 - the Repository driven queries are now from ElasticSearch and the model is no longer usable.

If I run two different end points - these will work. 1 thing that irks me but I couldn't find a way to optimize it efficiently: OutputFormat class that needs to iterate in order to normalize the output (ElasticSearch toss into their output _source on each object returned which is a nuissance to the end user).

Questions:

Given the working structure, was the best approach of MVC in order to achieve uniformity and also keep SOLID + DRY? I clearly fail on the dry since I had to make 2 controllers.

As mentioned above - I'm going to insert a third storage source, so combining and refactoring this would need to be extensible for two future tweaks:

  1. future storage engine to be able to plug in and play.
  2. If I have a controller - that may require to utilize all 3 different storage engine for different purpose - the repository (or service) method makes the most sense since the controller feeds in input and receives output.

However, given that a Data Model shouldn't know about databases because it renders it inefficient, I'm uncertain about optimizing the above code for it to conform to that concept.

Note: I've taken an extensive read to @Elias van Ootegem's post, however I'm not 100% sure how to clean and optimize the above code

\$\endgroup\$
1
\$\begingroup\$

I think your current solution is already right on track, you just need to take it further along. To combine the currently two controllers into one, lets write the code we wish we had. You want your controller to know almost nothing about the source the data comes from. Since your two actions are similar in interface already, lets start with this:

class MusicController extends BaseController
{
    public function getPopularListAction($device, $limit, $offset)
    {
        $popular = $dataProvider->getPopular($device, $limit, $offset);

        return $popular;
    }
}

Now what is $dataProvider? This could either be the ElasticSearch or the relational DB oder the NoSql version of a MusicRepository. To do that you would have all those Repositories implement the same interface e.g.

interface MusicRepositoryInterface
{
    public function getPopular($device, $limit, $offset);
}

class ElasticSearchRepository implements MusicRepositoryInteface {}
class NoSqlMusicRepository implements MusicRepositoryInterface {}

This is currently not the case and it looks like something you could improve on. If it were the case, you could then decide in your controller which of those Repositories to choose. This might either be an option in your api, or something you can do in a service container before the request is handled. I dont know your requirements here. (See http://silex.sensiolabs.org/doc/services.html for service container, dependency injection ...)

So e.g.

class MusicController extends BaseController
{
    // either alias one of those repos to music_repository
    // or decide based on an api flag or whatever may be the case
    $dataProvider = $this->app['music_repository'];
    // ...
}

If enforcing the same interface for all the repositories is not what you´d like to choose, you could also introduce another layer of abstraction through some kind of mediator object that decides the repository for you and does the work to provide a consistent output for the controller.

Instead of returning an array from your repositories, you could instead return objects relevant to your domain. E.g. an Album object, that knows nothing of databases and the sort. Each of the repositories can then populate such an object, and the controller can use another layer of abstraction to turn the array ob Albums into arrays/json.

Update

The Eloquent package is implementing the ActiveRecord pattern which is certainly useful. However it means that you Album comes with a lot of stuff that is not relevant to an Album per se. It inherits all the Active Record methods provided by Eloquent. I find when I have the need to swap backends, the ActiveRecord pattern is less useful. The object relevant to you domain could simply look like

class Album
{
    public function getTitle() { return $this->title; }
    // more properties, getters, setters, but no superclasses or save methods
}

Not knowing anything about any datastore. Your different Repositories / backends would then simply populate this entity. Compare to the Doctrine Project which is more of a Data Mapper pattern implementation.

Update 2

As posted above, "pluggin in" a different backend is as simple as getting the relevant repository from the service locator. You can do this by either aliasing the "selected" one to music_repository, or by selecting it on the fly in the controller

class MusicController extends BaseController
{
    public function getPopularListAction($device, $limit, $offset)
    {
        // either alias one of those repos to music_repository
        $dataProvider = $this->app['music_repository'];

        // or decide based on an api flag or whatever may be the case
        // simply register all of your backends in the container
        // music_repository.nosql
        // music_repository.elasticsearch
        // music_repository.whatever
        // again see here http://silex.sensiolabs.org/doc/services.html for docs
        $selectedBackend = 'nosql'; // your logic to decide on a backend
        $dataProvider = $this->app['music_repository.'.$selectedBackend];
    }
}

How you plan to decide which backend to use is unclear to me from your provided information.

\$\endgroup\$
  • \$\begingroup\$ can you please elaborate on the Object relevant to the domain please. which is in essence a way to data model without knowing the source of where the data comes from. That is part of what I am looking for to unify and understand. tks \$\endgroup\$ – azngunit81 Jan 6 '15 at 21:50
  • \$\begingroup\$ Updated with some hints to the Active Record and Data Mapper patterns which might help. \$\endgroup\$ – user1777136 Jan 7 '15 at 10:50
  • \$\begingroup\$ your updated active record/data mapper doesn't explain how to plugin the two backend or how to swap them out while keeping "album" intact over the other places. \$\endgroup\$ – azngunit81 Jan 8 '15 at 13:40
  • \$\begingroup\$ Updated with a more specific example on selecting the backend. \$\endgroup\$ – user1777136 Jan 8 '15 at 16:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.