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

Business Platform

By Nastasi Christian

Business Platform

  • 144

More from Nastasi Christian