From bc958c7b63d75a4322a34c4082ef2c5267bf0afd Mon Sep 17 00:00:00 2001 From: Chris Holland Date: Fri, 22 Dec 2017 13:51:47 -0800 Subject: [PATCH] refactored to further minimize couplings between tests and production code. --- src/AppBundle/Entity/AppUser.php | 3 ++ src/AppBundle/Repository/RideRepository.php | 5 ++-- src/AppBundle/Repository/UserRepository.php | 5 ++-- src/AppBundle/Service/RideService.php | 19 +++++++----- src/AppBundle/Service/UserService.php | 18 ++++++++--- tests/AppBundle/AppTestCase.php | 4 +-- tests/AppBundle/LocationRepositoryTest.php | 8 +++-- tests/AppBundle/RideEventRepositoryTest.php | 33 +++++++++++++-------- tests/AppBundle/RideRepositoryTest.php | 18 +++++++---- tests/AppBundle/UserRepositoryTest.php | 20 ++++++++----- tests/AppBundle/UserServiceTest.php | 4 +-- 11 files changed, 91 insertions(+), 46 deletions(-) diff --git a/src/AppBundle/Entity/AppUser.php b/src/AppBundle/Entity/AppUser.php index 79e5f9e..e0c31f7 100644 --- a/src/AppBundle/Entity/AppUser.php +++ b/src/AppBundle/Entity/AppUser.php @@ -59,6 +59,9 @@ public function __construct($firstName, $lastName) $this->roles = new ArrayCollection(); } + /** + * @return Uuid + */ public function getId() { return $this->id; diff --git a/src/AppBundle/Repository/RideRepository.php b/src/AppBundle/Repository/RideRepository.php index 119a93f..ec0fd93 100644 --- a/src/AppBundle/Repository/RideRepository.php +++ b/src/AppBundle/Repository/RideRepository.php @@ -5,6 +5,7 @@ use AppBundle\Entity\AppLocation; use AppBundle\Entity\AppUser; use AppBundle\Entity\Ride; +use Ramsey\Uuid\Uuid; class RideRepository extends AppRepository { @@ -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' diff --git a/src/AppBundle/Repository/UserRepository.php b/src/AppBundle/Repository/UserRepository.php index 66e0cd8..addb4be 100644 --- a/src/AppBundle/Repository/UserRepository.php +++ b/src/AppBundle/Repository/UserRepository.php @@ -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' diff --git a/src/AppBundle/Service/RideService.php b/src/AppBundle/Service/RideService.php index 5bc2a23..89c4cb9 100644 --- a/src/AppBundle/Service/RideService.php +++ b/src/AppBundle/Service/RideService.php @@ -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, @@ -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() @@ -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() @@ -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 + ); + } } diff --git a/src/AppBundle/Service/UserService.php b/src/AppBundle/Service/UserService.php index 1262b9c..94fcabf 100644 --- a/src/AppBundle/Service/UserService.php +++ b/src/AppBundle/Service/UserService.php @@ -5,6 +5,7 @@ use AppBundle\Entity\AppRole; use AppBundle\Entity\AppUser; use AppBundle\Repository\UserRepository; +use Ramsey\Uuid\Uuid; class UserService { @@ -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) @@ -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); + } } diff --git a/tests/AppBundle/AppTestCase.php b/tests/AppBundle/AppTestCase.php index 6fe9bb8..b58e16f 100644 --- a/tests/AppBundle/AppTestCase.php +++ b/tests/AppBundle/AppTestCase.php @@ -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); @@ -168,7 +168,7 @@ protected function getSavedHomeLocation() * @param $userId * @return AppUser */ - protected function getUserById($userId) + protected function getServiceUserById($userId) { return $this->userService->getUserById($userId); } diff --git a/tests/AppBundle/LocationRepositoryTest.php b/tests/AppBundle/LocationRepositoryTest.php index a2ef341..d1edfd9 100644 --- a/tests/AppBundle/LocationRepositoryTest.php +++ b/tests/AppBundle/LocationRepositoryTest.php @@ -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)); } @@ -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)); } @@ -63,4 +63,8 @@ private function getSavedLocation() return $homeLocation; } + protected function getOrCreateLocation(AppLocation $lookupLocation) + { + return $this->locationRepository->getLocation($lookupLocation); + } } diff --git a/tests/AppBundle/RideEventRepositoryTest.php b/tests/AppBundle/RideEventRepositoryTest.php index fa49016..44a5ac9 100644 --- a/tests/AppBundle/RideEventRepositoryTest.php +++ b/tests/AppBundle/RideEventRepositoryTest.php @@ -2,6 +2,7 @@ namespace Tests\AppBundle; +use AppBundle\Entity\AppUser; use AppBundle\Entity\Ride; use AppBundle\Entity\RideEvent; use AppBundle\Entity\RideEventType; @@ -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())); } @@ -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())); } @@ -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; } /** @@ -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 + ); + } } diff --git a/tests/AppBundle/RideRepositoryTest.php b/tests/AppBundle/RideRepositoryTest.php index c8fdf4c..96ea2a3 100644 --- a/tests/AppBundle/RideRepositoryTest.php +++ b/tests/AppBundle/RideRepositoryTest.php @@ -4,6 +4,7 @@ use AppBundle\Entity\AppUser; use AppBundle\Entity\Ride; +use Ramsey\Uuid\Uuid; /** * Class RideRepositoryTest @@ -51,12 +52,10 @@ 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)); } @@ -64,7 +63,7 @@ public function testAssignDriverToRide() /** * @return Ride */ - private function getRideWithDestination() + protected function getRideWithDestination() { $ride = $this->getSavedRide(); @@ -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); + } } diff --git a/tests/AppBundle/UserRepositoryTest.php b/tests/AppBundle/UserRepositoryTest.php index 4f8f1ec..af97843 100644 --- a/tests/AppBundle/UserRepositoryTest.php +++ b/tests/AppBundle/UserRepositoryTest.php @@ -3,6 +3,7 @@ namespace Tests\AppBundle; use AppBundle\Entity\AppRole; +use AppBundle\Entity\AppUser; use AppBundle\Exception\DuplicateRoleAssignmentException; class UserRepositoryTest extends AppTestCase @@ -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)); @@ -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())); @@ -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); + } } diff --git a/tests/AppBundle/UserServiceTest.php b/tests/AppBundle/UserServiceTest.php index 9d2ecf8..dc93f5a 100644 --- a/tests/AppBundle/UserServiceTest.php +++ b/tests/AppBundle/UserServiceTest.php @@ -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)); } @@ -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)); }