Business Platform

Report

What we found

God Objects
No Authentication
Copy and paste everywhere
A lot of passtrough methods
Core errors forced at 401 (a lot of them)
No proper error handling
No input validation
No status codes (always 200)
Domain logic fragmented everywhere with an unclear separation

Issues (1)

Dependencies injected through '$container->get()'
No type hinting
No documentation
No features mapping
2 different api version (no idea on what was used)
No domain know how
Prototype quality
Poor architectural design
PHP 7.0
Symfony 3.1.6

Issues (2)

    4471 | src/AppBundle/Controller/Platform/ContentController.php
    2858 | src/Musement/ApiClient/MusementApiClient.php
    2231 | src/AppBundle/Controller/Api/ApiController.php
    1992 | src/AppBundle/Controller/Platform/UpdateEventController.php
    1591 | src/AppBundle/Controller/Api/v2/EventDateController.php
    1512 | src/AppBundle/Controller/Api/v2/SupplierController.php
    1346 | src/AppBundle/Controller/Api/v2/EventController.php
    1288 | src/AppBundle/Controller/Platform/CreateEventController.php
     574 | src/AppBundle/Command/EventUpdateRequestsCommand.php
     525 | src/AppBundle/Command/OrdersExcelCreationCommand.php

GOD objects

Just few examples

No Authentication

if (!isset($headers['tk'])) {
   $httpResponse->setContent(
      \json_encode((object) [
         'data' => [
            'error' => (object) [
               'code' => 'TEXT_OK', 
               'message' => 'Wrong arguments.'
            ]
         ]
      ])
   );

   return $httpResponse;
}

Copy and paste (1)

$client = $this->getApiClient();
$httpResponse = new JsonResponse();
// Allow all websites
$httpResponse->headers->set('Access-Control-Allow-Headers', 'Access-Control-Allow-Origin,Content-Type,tk,role');
$httpResponse->headers->set('Access-Control-Allow-Methods', 'GET,OPTIONS');
$httpResponse->headers->set('Access-Control-Allow-Origin', '*');

$headers = $request->headers->all();

if (!isset($headers['tk'])) {
    $httpResponse->setContent(\json_encode((object) ['data' => ['error' => (object) ['code' => 'WRONG_ARGS', 'message' => 'Wrong arguments.']]]));

    return $httpResponse;
}

$response = $client->getCities();

$result = (object) ['data' => (array) $response];
$httpResponse->setContent(\json_encode($result));

return $httpResponse;

Copy and paste (2)

$client = $this->getApiClient();
$httpResponse = new JsonResponse();
// Allow all websites
$httpResponse->headers->set('Access-Control-Allow-Headers', 'Access-Control-Allow-Origin,Content-Type,tk,role');
$httpResponse->headers->set('Access-Control-Allow-Methods', 'PATCH,OPTIONS');
$httpResponse->headers->set('Access-Control-Allow-Origin', '*');

$headers = $request->headers->all();

if (!isset($headers['tk'])) {
    $httpResponse->setContent(\json_encode((object) ['data' => ['error' => (object) ['code' => 'WRONG_ARGS', 'message' => 'Wrong arguments.']]]));

    return $httpResponse;
}

$response = $client->getCountries();

$result = (object) ['data' => (array) $response];
$httpResponse->setContent(\json_encode($result));

return $httpResponse;

Response always 200

list($response, $statusCode) = $client->getLoggedSupplierTicketHolders($accessToken[0]);

if (200 !== $statusCode) {
    $result = (object) [
        'error' => (object) [
            'message' => $response, 
            'http_code' => $statusCode
        ]
    ];
} else {
    $result = (object) ['data' => $response];
}

return new JsonResponse($result);

Passtrough methods (1)

public function getActivityDatesV2($eventId, $params, $currency = 'USD')
{
    $endpoint = \sprintf('activities/%s/dates', $eventId);

    try {
        $response = $this->request('GET', $endpoint.'?'.\http_build_query($params), [
            'headers' => [
                'Accept' => 'application/json',
                'X-Musement-Version' => '3.2.0',
                'X-Musement-Currency' => $currency,
            ],
        ]);
    } catch (Exception $e) {
        return [$e->getCode().' | '.$e->getResponse()->getBody()->getContents(), 401];
    }

    return [GetEventDatesMapper::map($response), $response->getStatusCode()];
}

Passtrough methods (2)

public function activityDatesTimeslotsPatchV2($accessToken, $eventId, $day, $time, $payload)
{
    $endpoint = \sprintf('activities/%s/dates/%s/timeslots/%s', $eventId, $day, $time);

    try {
        $response = $this->request('PATCH', $endpoint, [
            'headers' => [
                'Accept' => 'application/json',
                'X-Musement-Version' => '3.2.2',
                'Authorization' => 'Bearer '.$accessToken,
            ],
            'json' => $payload,
        ]);
    } catch (Exception $e) {
        return [$e->getCode().' | '.$e->getResponse()->getBody()->getContents(), $e->getCode()];
    }

    return [ActivityDatesPatchMapper::map($response), $response->getStatusCode()];
}

Spaghetti code

class EventSentToOnline extends Event
{
    public function eventSentToOnline(EntityManager $entityManager, Container $container)
    {
        $this->notificationRepository = $entityManager->getRepository('AppBundle:Notification');
        $this->eventRepository = $entityManager->getRepository('AppBundle:Event');
        $this->setpRepository = $entityManager->getRepository('AppBundle:Step');
    
        $this->mailer = $container->get('mailer');
        $this->templating = $container->get('templating');
    
        $event = $this->eventRepository->find($this->eventId);
    
        $notification = new Notification();
    
        $notification->setMessage($event->getTitle().' is now Online');
        $notification->setActivityId($this->eventId);
        $notification->setStatus('UNSEEN');
        $notification->setType('EVENT_ONLINE');
        $notification->setUserId($event->getSupplierId());
        $notification->setUserType('supplier');
        $notification->setCreatedAt(new \DateTime('now'));
    
        $entityManager->persist($notification);
        $entityManager->flush();
    }

Dependencies injected to a method

Dependency getted from the container and never used

Hardcoded strings

Persistence inside an event???

What was done

Requirements

In the last 2 years we tried to build the platform but we fail cause several issues and poor software quality.

Improve and stabilize it cause it will be very important in the near future.

Approach taken

Keep what works
Refactor what is affected by new features
Segregation between new and old code

(Strangler pattern)

Biggest improvement

Centralized Authentication (with JWT)

class JwtAuthMiddleware
{
    public function onKernelRequest(GetResponseEvent $event) { /* ... */ }
}

Centralized exception handler

private function renderValidationError(ValidationError $exception): JsonResponse
{
    $response = JsonResponse::create(
        [
            'message' => $exception->getMessage(),
            'type' => $exception->getType(),
        ],
        Response::HTTP_BAD_REQUEST
    );

    return $response;
}


private function renderResourceNotFound(ResourceNotFound $exception): JsonResponse
{
    return  JsonResponse::create(
        [ 'message' => $exception->getMessage() ],
        Response::HTTP_NOT_FOUND
    );
}

private function renderUnauthorized(Unauthorized $exception): JsonResponse
{
    return JsonResponse::create(
        [ 'message' => $exception->getMessage() ],
        Response::HTTP_UNAUTHORIZED
    );
}

Domain Exceptions

Core proxy

catch-all:
  path: /api/v3/{req}
  defaults: { _controller: business_platform.proxy.core.controller }
  requirements:
    req: ".+"

Separation between Frontend and Backend

Explicit class dependecies

public function __construct(
        CredentialChecker $credentialChecker,
        TokenFactory $tokenFactory,
        EventDispatcher $eventDispatcher
    )

Strong type checking and data safety through immutable value objects

final class Email
{
    /* ... */
    public function __construct(string $email)
    {
        $this->assertNotEmpty($email);
        $this->assertIsEmail($email);

        $this->email = $email;
    }

final class DateRange implements \IteratorAggregate
{
    /* ... */
    public function __construct(Date $from, Date $to)
    {
        if ($from->greaterThan($to)) {
            throw new ValidationError(self::class, 'Invalid date range');
        }

        $this->from = $from;
        $this->to = $to;
    }

"require": {
    "nelmio/cors-bundle": "^1.5"
}

Centralized CORS

Consistent architecture & Naming convention

Controllers
Get the input and call the use case. Format the response

Use cases
Responsible for the domain business logic. Feature-based.

Mappers
Transform raw data into domain value objects

(and vice-versa)

Repositories
Responsible for the domain entities persistence

Consistent architecture & Naming convention

Entities and Value Objects
Autovalidating and almost immutables.
Describes the domain data
(except for specific cases)

Services

Contains logic shared between several use cases.

Adapters

Connect the application to an external domain (ex: Core)

Handlers

Executes code after an event occured

Example 1

Clean and expressive code

/* CreateUserSessionController */
public function __invoke(Request $request)
{
    $json = $this->parseJson($request);
    
    $credentials = $this->getCredentials($json);
    
    $token = $this->userSession->create($credentials);
    
    return JsonResponse::create([
        'token' => (string) $token,
        'role' => (string) $token->role(),
        'uuid' => $token->userId()->value(),
    ], JsonResponse::HTTP_CREATED);
}

private function getCredentials($json): EmailAndPasswordCredentials
{
    $credentials = new EmailAndPasswordCredentials(
        new Email($json[self::FIELD_EMAIL] ?? ''),
        new Password($json[self::FIELD_PASSWORD] ?? '')
    );

    return $credentials;
}

Get and validate the input

Call the use case

Create the response

Clean and expressive code

/* CreateUserSession */
public function create(Credentials $credentials): Token
{
    // Throws UserNotFound and InvalidPassword
    $user = $this->credentialChecker->check($credentials);

    $this->eventDispatcher->dispatch(new UserSessionCreated($user));

    return $this->tokenFactory->createToken($user);
}

No primitives as arguments. Clear contract

Check the credentials

Trigger the event

Create and returns the Access Token

Example 2

Clean and expressive code

/* GetSeasonsController */
public function __invoke(Request $request, string $id): JsonResponse
{
    // Throws ValidationError
    $activityId = ActivityId::fromString($id);

    // Throws ResourceNotFound
    $seasons = $this->getSeasons($activityId);

    $response = JsonResponse::create(
        $this->serializeSeasons($seasons)
    );

    return $response;
}

Invokable force Single Responsible controllers

Get and validate the input

Get the seasons

Create the response

Layered Architecture

Distro

Bundle

Domain

Distro (infrastructure)
Contains the framework and the legacy code.

It provides the configuration to all the external resources (like DB, cache, remote services, etc...)

Layered Architecture

Distro

Bundle

Domain

Bundle (Application)
Contains all the business platform related code.

It knows about Symfony and exposes several services and routes.

Layered Architecture

Distro

Bundle

Domain

Domain
Contains all the Musement domain concept used in the Business Platform.

It's a stable library that could be shared between several application.

Major releases

Seasons
Splitting in repos
Environment upgrade (PHP 7.2, Symfony 4.2)
JWT Authentication

Followed principles

Domain Driven Design
Clean Architecture
Clean Code
SOLID

What could be improved

Possible improvements

Increase the test coverage
Extract libraries in order to be shared between projects
A better domain definition
Full refactor the platform
Remove all the legacy code