Skip to content

Commit

Permalink
refactored to further minimize couplings between tests and production…
Browse files Browse the repository at this point in the history
… code.
  • Loading branch information
Chris Holland committed Dec 22, 2017
1 parent 65926ab commit bc958c7
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 46 deletions.
3 changes: 3 additions & 0 deletions src/AppBundle/Entity/AppUser.php
Expand Up @@ -59,6 +59,9 @@ public function __construct($firstName, $lastName)
$this->roles = new ArrayCollection();
}

/**
* @return Uuid
*/
public function getId()
{
return $this->id;
Expand Down
5 changes: 3 additions & 2 deletions src/AppBundle/Repository/RideRepository.php
Expand Up @@ -5,6 +5,7 @@
use AppBundle\Entity\AppLocation;
use AppBundle\Entity\AppUser;
use AppBundle\Entity\Ride;
use Ramsey\Uuid\Uuid;

class RideRepository extends AppRepository
{
Expand All @@ -15,10 +16,10 @@ public function assignDestinationToRide(Ride $ride, AppLocation $destination)
}

/**
* @param $id
* @param Uuid $id
* @return Ride
*/
public function getRideById($id)
public function getRideById(Uuid $id)
{
return $this->em->createQuery(
'select r from E:Ride r where r.id = :id'
Expand Down
5 changes: 3 additions & 2 deletions src/AppBundle/Repository/UserRepository.php
Expand Up @@ -5,14 +5,15 @@
use AppBundle\Entity\AppRole;
use AppBundle\Entity\AppUser;
use AppBundle\Exception\DuplicateRoleAssignmentException;
use Ramsey\Uuid\Uuid;

class UserRepository extends AppRepository
{
/**
* @param $userId
* @param Uuid $userId
* @return AppUser
*/
public function getUserById($userId)
public function getUserById(Uuid $userId)
{
return $this->em->createQuery(
'select u from E:AppUser u where u.id = :userId'
Expand Down
19 changes: 12 additions & 7 deletions src/AppBundle/Service/RideService.php
Expand Up @@ -64,11 +64,7 @@ public function acceptRide(Ride $ride, AppUser $driver)
$this->validateUserHasDriverRole($driver);
$this->validateRideIsRequested($ride);

$this->rideEventRepository->markRideStatusByActor(
$ride,
$driver,
RideEventType::accepted()
);
$this->markRide($ride, $driver, RideEventType::accepted());

$this->rideRepository->assignDriverToRide(
$ride,
Expand All @@ -84,7 +80,7 @@ public function markRideInProgress(Ride $acceptedRide, AppUser $driver)
$this->validateUserHasDriverRole($driver);
$this->validateAttemptingDriverIsAssignedDriver($acceptedRide, $driver);

$this->rideEventRepository->markRideStatusByActor(
$this->markRide(
$acceptedRide,
$driver,
RideEventType::inProgress()
Expand All @@ -97,7 +93,7 @@ public function markRideCompleted(Ride $rideInProgress, AppUser $driver)
$this->validateAttemptingDriverIsAssignedDriver($rideInProgress, $driver);
$this->validateRideIsInProgress($rideInProgress);

$this->rideEventRepository->markRideStatusByActor(
$this->markRide(
$rideInProgress,
$driver,
RideEventType::completed()
Expand Down Expand Up @@ -164,4 +160,13 @@ private function validateRideIsInProgress(Ride $rideInProgress)
throw new RideLifeCycleException();
}
}

private function markRide(Ride $ride, AppUser $driver, RideEventType $status)
{
$this->rideEventRepository->markRideStatusByActor(
$ride,
$driver,
$status
);
}
}
18 changes: 14 additions & 4 deletions src/AppBundle/Service/UserService.php
Expand Up @@ -5,6 +5,7 @@
use AppBundle\Entity\AppRole;
use AppBundle\Entity\AppUser;
use AppBundle\Repository\UserRepository;
use Ramsey\Uuid\Uuid;

class UserService
{
Expand All @@ -30,17 +31,17 @@ public function newUser($firstName, $lastName)
}

/**
* @param int $userId
* @param Uuid $userId
* @return AppUser
*/
public function getUserById($userId)
public function getUserById(Uuid $userId)
{
return $this->userRepository->getUserById($userId);
}

public function makeUserDriver(AppUser $user)
{
$this->userRepository->assignRoleToUser($user, AppRole::driver());
$this->assignRole($user, AppRole::driver());
}

public function isDriver(AppUser $user)
Expand All @@ -50,11 +51,20 @@ public function isDriver(AppUser $user)

public function makeUserPassenger(AppUser $user)
{
$this->userRepository->assignRoleToUser($user, AppRole::passenger());
$this->assignRole($user, AppRole::passenger());
}

public function isPassenger(AppUser $user)
{
return $user->hasRole(AppRole::passenger());
}

/**
* @param AppUser $user
* @param $role
*/
protected function assignRole(AppUser $user, AppRole $role)
{
$this->userRepository->assignRoleToUser($user, $role);
}
}
4 changes: 2 additions & 2 deletions tests/AppBundle/AppTestCase.php
Expand Up @@ -146,7 +146,7 @@ protected function getNewDriverWithName($first, $last)
protected function getSavedRide()
{
$ridePassenger = $this->getNewPassenger();
$this->savedPassenger = $this->getUserById($ridePassenger);
$this->savedPassenger = $this->getServiceUserById($ridePassenger->getId());
$departure = $this->getSavedHomeLocation();
$ride = new Ride($this->savedPassenger, $departure);
$this->rideRepository->save($ride);
Expand All @@ -168,7 +168,7 @@ protected function getSavedHomeLocation()
* @param $userId
* @return AppUser
*/
protected function getUserById($userId)
protected function getServiceUserById($userId)
{
return $this->userService->getUserById($userId);
}
Expand Down
8 changes: 6 additions & 2 deletions tests/AppBundle/LocationRepositoryTest.php
Expand Up @@ -33,7 +33,7 @@ public function testGetExistingLocationByLatLong()
$savedLocation = $this->getSavedLocation();
$lookupLocation = AppLocation::cloneFrom($savedLocation);

$retrievedLocation = $this->locationRepository->getLocation($lookupLocation);
$retrievedLocation = $this->getOrCreateLocation($lookupLocation);

self::assertTrue($retrievedLocation->equals($savedLocation));
}
Expand All @@ -43,7 +43,7 @@ public function testCreateAndGetNewLocation()
$this->getSavedLocation();
$workLocation = new AppLocation(self::WORK_LOCATION_LAT, self::WORK_LOCATION_LONG);

$retrievedLocation = $this->locationRepository->getLocation($workLocation);
$retrievedLocation = $this->getOrCreateLocation($workLocation);

self::assertTrue($retrievedLocation->equals($workLocation));
}
Expand All @@ -63,4 +63,8 @@ private function getSavedLocation()
return $homeLocation;
}

protected function getOrCreateLocation(AppLocation $lookupLocation)
{
return $this->locationRepository->getLocation($lookupLocation);
}
}
33 changes: 20 additions & 13 deletions tests/AppBundle/RideEventRepositoryTest.php
Expand Up @@ -2,6 +2,7 @@

namespace Tests\AppBundle;

use AppBundle\Entity\AppUser;
use AppBundle\Entity\Ride;
use AppBundle\Entity\RideEvent;
use AppBundle\Entity\RideEventType;
Expand Down Expand Up @@ -59,9 +60,7 @@ public function testRideIsCurrentlyRequested()
{
$this->getSavedRequestedRideEvent();

$lastEventForRide = $this->rideEventRepository->getLastEventForRide(
$this->savedRide
);
$lastEventForRide = $this->getLastEvent($this->savedRide);

self::assertTrue($lastEventForRide->is(RideEventType::requested()));
}
Expand Down Expand Up @@ -93,14 +92,12 @@ public function testRideIsCurrentlyRejected()

public function testMarkRideAsStatus()
{
$this->rideEventRepository->markRideStatusByActor(
$this->markRide(
$this->savedRide,
$this->savedPassenger,
RideEventType::requested()
);
$lastEventForRide = $this->rideEventRepository->getLastEventForRide(
$this->savedRide
);
$lastEventForRide = $this->getLastEvent($this->savedRide);

self::assertTrue($lastEventForRide->is(RideEventType::requested()));
}
Expand All @@ -110,13 +107,11 @@ public function testMarkRideAsStatus()
*/
private function getSavedRequestedRideEvent()
{
$rideEvent = $this->rideEventRepository->markRideStatusByActor(
return $this->markRide(
$this->savedRide,
$this->savedPassenger,
$this->requestedType
);

return $rideEvent;
}

/**
Expand All @@ -133,13 +128,25 @@ private function assertLastEventIsOfType(RideEventType $eventTypeToAssert)
$eventTypeToAssert
));

$lastEventForRide = $this->rideEventRepository->getLastEventForRide(
$this->savedRide
);
$lastEventForRide = $this->getLastEvent($this->savedRide);

self::assertFalse($lastEventForRide->is(RideEventType::requested()));
self::assertTrue($lastEventForRide->is($eventTypeToAssert));

return $lastEventForRide;
}

protected function getLastEvent(Ride $ride)
{
return $this->rideEventRepository->getLastEventForRide($ride);
}

protected function markRide(Ride $ride, AppUser $passenger, RideEventType $status)
{
return $this->rideEventRepository->markRideStatusByActor(
$ride,
$passenger,
$status
);
}
}
18 changes: 13 additions & 5 deletions tests/AppBundle/RideRepositoryTest.php
Expand Up @@ -4,6 +4,7 @@

use AppBundle\Entity\AppUser;
use AppBundle\Entity\Ride;
use Ramsey\Uuid\Uuid;

/**
* Class RideRepositoryTest
Expand Down Expand Up @@ -51,20 +52,18 @@ public function testAssignDriverToRide()
{
/** @var AppUser $driver */
$driver = $this->getSavedUserWithName('Jamie', 'Isaacs');

$rideWithDestination = $this->getRideWithDestination();

$this->rideRepository->assignDriverToRide($rideWithDestination, $driver);

$retrievedRide = $this->rideRepository->getRideById($rideWithDestination->getId());
$retrievedRide = $this->getRideById($rideWithDestination->getId());

self::assertTrue($retrievedRide->isDrivenBy($driver));
}

/**
* @return Ride
*/
private function getRideWithDestination()
protected function getRideWithDestination()
{
$ride = $this->getSavedRide();

Expand All @@ -74,8 +73,17 @@ private function getRideWithDestination()
);

/** @var Ride $retrievedRide */
$retrievedRide = $this->rideRepository->getRideById($ride->getId());
$retrievedRide = $this->getRideById($ride->getId());

return $retrievedRide;
}

/**
* @param Uuid $id
* @return Ride
*/
protected function getRideById(Uuid $id)
{
return $this->rideRepository->getRideById($id);
}
}
20 changes: 13 additions & 7 deletions tests/AppBundle/UserRepositoryTest.php
Expand Up @@ -3,6 +3,7 @@
namespace Tests\AppBundle;

use AppBundle\Entity\AppRole;
use AppBundle\Entity\AppUser;
use AppBundle\Exception\DuplicateRoleAssignmentException;

class UserRepositoryTest extends AppTestCase
Expand All @@ -23,7 +24,7 @@ public function testGetUserById()
{
$savedUser = $this->getSavedUser();

$retrievedUser = $this->userRepository->getUserById($savedUser->getId());
$retrievedUser = $this->getServiceUserById($savedUser->getId());

self::assertSame($savedUser->getId(), $retrievedUser->getId());
self::assertTrue($savedUser->is($retrievedUser));
Expand All @@ -43,10 +44,10 @@ public function testUserCanHaveBothRoles()
{
$savedUser = $this->getSavedUser();

$this->userRepository->assignRoleToUser($savedUser, AppRole::driver());
$this->userRepository->assignRoleToUser($savedUser, AppRole::passenger());
$this->assignRoleToUser($savedUser, AppRole::driver());
$this->assignRoleToUser($savedUser, AppRole::passenger());

$retrievedUser = $this->userRepository->getUserById($savedUser->getId());
$retrievedUser = $this->getServiceUserById($savedUser->getId());

self::assertTrue($retrievedUser->hasRole(AppRole::driver()));
self::assertTrue($retrievedUser->hasRole(AppRole::passenger()));
Expand All @@ -56,19 +57,24 @@ public function testDuplicateRoleAssignmentThrows()
{
$savedUser = $this->getSavedUser();

$this->userRepository->assignRoleToUser($savedUser, AppRole::driver());
$this->assignRoleToUser($savedUser, AppRole::driver());
$this->expectException(DuplicateRoleAssignmentException::class);

$this->userRepository->assignRoleToUser($savedUser, AppRole::driver());
$this->assignRoleToUser($savedUser, AppRole::driver());
}

private function assertUserHasExpectedRole(AppRole $role)
{
$savedUser = $this->getSavedUser();

$this->userRepository->assignRoleToUser($savedUser, $role);
$this->assignRoleToUser($savedUser, $role);
$retrievedUser = $this->userRepository->getUserById($savedUser->getId());

self::assertTrue($retrievedUser->hasRole($role));
}

protected function assignRoleToUser(AppUser $user, AppRole $role)
{
$this->userRepository->assignRoleToUser($user, $role);
}
}
4 changes: 2 additions & 2 deletions tests/AppBundle/UserServiceTest.php
Expand Up @@ -17,7 +17,7 @@ public function testMakeUserDriver()
{
$savedUser = $this->getSavedUser();
$this->makeUserDriver($savedUser);
$retrievedUser = $this->getUserById($savedUser->getId());
$retrievedUser = $this->getServiceUserById($savedUser->getId());

self::assertTrue($this->userService->isDriver($retrievedUser));
}
Expand All @@ -26,7 +26,7 @@ public function testMakeUserPassenger()
{
$savedUser = $this->getSavedUser();
$this->makeUserPassenger($savedUser);
$retrievedUser = $this->getUserById($savedUser->getId());
$retrievedUser = $this->getServiceUserById($savedUser->getId());

self::assertTrue($this->isPassenger($retrievedUser));
}
Expand Down

0 comments on commit bc958c7

Please sign in to comment.