isValidLogin() and login() belong to an application service IMO. They have nothing to do with a repository, whose responsibility is to provide collection-like interface for domain objects or aggregate root.
May I ask why you want to add those two methods in a repository object?
moon0326 said:
isValidLogin() and login() belong to an application service IMO. They have nothing to do with a repository, whose responsibility is to provide collection-like interface for domain objects or aggregate root.
May I ask why you want to add those two methods in a repository object?
I don't exactly. It was just the example I was looking at was doing it like that and I thought that it was actually a good approach as it makes testing small portions of the code easier.
If separating those two methods out (I'm not sure why..and from where?) makes testing easier, they should belong to other objects, not a repository IMO.
I don't even think 'all' method should be in the interface in most cases. How many times do we actually need 'all' the records in a persistence layer?
As you may know, "Repository" is a design pattern:
http://martinfowler.com/eaaCatalog/repository.html
http://code.tutsplus.com/tutorials/the-repository-design-pattern--net-35804
If you were to follow the pattern (which you should), you wouldn't put anything that doesn't directly relate to getting/setting raw data. As @moon0326 said, those login methods shouldn't really be there for that example either.
When you're thinking of validation, you're actually thinking of two different kinds of validation:
- Business logic: Is this a valid phone number? Is the password strong enough? Is the email address being used not already in the system?
- Data Storage Validation: Is this field value not null? Does this field value satisfy our unique constraint for this column? Is this field less than 40 characters in length?
The first belongs in application services, because our database doesn't give a damn about password strength. The second belongs as part of the repository, because those constraints are specific to how we're storing out data.
@thepsion5 // Are you suggesting we should add #2 validation in a repository?
moon0326 said:
@thepsion5 // Are you suggesting we should add #2 validation in a repository?
Correct. I feel #2 belongs in a repository because it's specific to how you're storing the data, and not your business rules.
Could you point me to a link where we add validation logic into a repository?
I've personally never seen a method in a repository object checking validation rules.
This is a simplified example of the technique I'm currently using (sans abstract classes, helper functions, etc):
Background: FooDatabaseValidator has assertValid(array $data) that throws an exception (containing the input and validation messages) if it fails the validation rules defined in its $rules property.
class EloquentFooRepository
{
public function __construct(FooModel $foo, FooDatabaseValidator $validator)
{
$this->validator = $validator;
$this->model = $foo;
}
public function create(array $data)
{
$this->validator->assertValid($data);
return $this->model->create($data);
}
public function findById($id)
{
$foo = $this->model->find($id);
if($foo == null) {
throw new FooResourceNotFoundException;
}
}
public function updateById($id, array $data)
{
$model = $this->findById($id);
return $this->updateEntity($model, $data);
}
public function updateEntity(Foo $entity, array $data = array())
{
if(count($data) > 0) {
$entity->fill($data);
}
$this->validator->assertValid($entity->toArray());
$entity->save();
}
}
Granted, it's not necessary to perform validation there - you could just put it all into a general validator and check it there. But the whole idea in using a repository is to separate your business logic from your storage logic, so to me it makes perfect sense to check db-specific constraints in a db repository. The business logic of your application doesn't care about the length of your email address, but your repository DOES care if it's being told to persist data that it isn't capable of saving correctly.
That's interesting. I've never used a validator in a repository.
The reasons are,
-
If you add validation logic into a repository, your code has to go through all the business logics and then hit the repository to find out whether they can be saved or not. Why do I need to load/initiate all the business objects if the values are not valid in the first place?
-
What if you have more than one validation rules? Do you add two rules into a repository?
-
How do you validate entities in your aggregate root if you ONLY validate the aggregate root in your repository?
I'm not expert on repositories or patterns, but I think validations belong to a service layer or entities depending on your need.
Just saw your updates. If your business logic doesn't care, but your repository does...what's that supposed to mean?
- My reasoning is that otherwise, you have to deal with one of the following scenarios:
- You create and validate all of your business entities, only to have it fail anyway due to a PDOException
- You successfully persist your entity, but because the email is 80 characters and your database field is only 60 characters, there's now an invalid entity in your database that will move through your application like a blood clot until it breaks something
- You muddy your business logic with rules that aren't actually business rules. The business doesn't care that your email address is 60 or fewer characters, it cares that it has a valid means of contacting you.
-
So far, my entities have been simple enough that using a single validation class wasn't limiting or awkward
-
It'd probably end up being a composite class with a validator for each entity, but probably I'd still inject it into the repository
I can't claim my approach is the best, but I'd rather my repositories reject an attempt to store something than store it in a corrupted form or force my domain logic to care about my specific storage implementations, and this felt like the cleanest way to accomplish both goals.
moon0326 said: Just saw your updates. If your business logic doesn't care, but your repository does...what's that supposed to mean?
You'll have to ask someone with more DDD experience or more brains than myself, I'm afraid. ;)
-
I think that's why we have an application service layer and a domain service layer. You don't have to perform validations in your entities. If I do validate inputs in my entities, then I could end up having multiple rules in my entity. That can be an issue. I really think this is up to us to decide.
-
Fair enough, but again...that's why we have service layers IMO.
-
This goes to #1 again.
Unless I have to access a repository to create a record directly from my controllers, I don't see why we need a validation rule in repositories in most cases.
thepsion5 said:
moon0326 said: Just saw your updates. If your business logic doesn't care, but your repository does...what's that supposed to mean?
You'll have to ask someone with more DDD experience or more brains than myself, I'm afraid. ;)
I was asking that because..from my understanding, repositories are business objects. A persistence mechanism (and its classes) being used in a repository is not a business object/layer, but a repository itself represents business logic.
moon0326 said:
I think that's why we have an application service layer and a domain service layer. You don't have to perform validations in your entities. If I do validate inputs in my entities, then I could end up having multiple rules in my entity. That can be an issue. I really think this is up to us to decide.
Fair enough, but again...that's why we have service layers IMO.
This goes to #1 again.
Unless I have to access a repository to create a record directly from my controllers, I don't see why we need a validation rule in repositories in most cases.
Good point. From that perspective, I could see how injecting a validator into my repository is kind of mixing my application and infrastructure layer. So in an ideal situation, you'd have:
- The domain layer tells the application to persist an entity
- The application layer checks to see if it can be persisted, throws some kind of exception if not
- The application layer passes it to the repository in the infrastructure layer
- The repository handles the persisting, assuming whatever it's been handed is valid
moon0326 said:
thepsion5 said:
moon0326 said: Just saw your updates. If your business logic doesn't care, but your repository does...what's that supposed to mean?
You'll have to ask someone with more DDD experience or more brains than myself, I'm afraid. ;)
I was asking that because..from my understanding, repositories are business objects. A persistence mechanism (and its classes) being used in a repository is not a business object/layer, but a repository itself represent business logic.
Excellent point. I suppose I've always been looking at it from the perspective of the interface being in the domain layer but the actual logic being in the application layer.
I wish I could say yes, but I'm still leaning these things as well.
-
I don't know. What I know so far is that an application or a domain service can/should access a repository. That means that an application can also ask a repository to persist an entity.
-
It looks like it depends on our applications. Some people prefer checking the rules in the entities, some people hate that. I think it's because some people have more complex business logics.
-
same thing as #1
-
That's how I understand it as of now.
moon0326 said:
I wish I could say yes, but I'm still leaning these things as well.
I don't know. What I know so far is that an application or a domain service can/should access a repository. That means that an application can also ask a repository to persist an entity.
It looks like it depends on our applications. Some people prefer checking the rules in the entities, some people hate that. I think it's because some people have more complex business logics.
same thing as #1
That's how I understand it as of now.
Thanks for the insight! I think I've gotten just a little better at comprehending the general concepts and separation between domain/app/infrastructure.
No problem! Glad to share share some of the things I'm learning!
Ok that was a whole lot to read ;)
The reason I originally though of this was because I was reading through some of the Wardrobe CMS code and noticed they were doing it. -- https://github.com/wardrobecms/core/blob/master/src/Wardrobe/Core/Repositories/DbUserRepository.php
I do not suppose anyone could give me an idea of what their post method may look like which handles rules/database insertion and what not.
I just feel like my UserController will be way to large.
For example
public function postAdd()
{
$rules = array(
'avatar' => array('image', 'mimes:jpeg,png,gif'),
'role' => array('required', 'exists:role,id'),
'username' => array('required', 'min:3', 'max:30', 'unique:user'),
'email' => array('required', 'email', 'unique:user'),
'first_name' => array('required', 'min:3', 'max:100'),
'last_name' => array('required', 'min:3', 'max:100'),
'password' => array('required', 'min:8', 'confirmed'),
'address' => array('required', 'min:3', 'max:100'),
'postcode' => array('required', 'integer'),
'suburb' => array('required', 'min:3', 'max:30'),
'state' => array('required', 'min:3', 'max:30'),
'country' => array('required', 'exists:country,id'),
'status' => array('required', 'in:'.implode(",", array_keys(User::getStatusTypes())))
);
$validator = Validator::make(Input::all(), $rules);
if ($validator->fails()) {
return Redirect::route('admin.users.add')
->withErrors($validator)
->withInput(Input::except('password'));
} else {
$user = new User();
if (Input::file('avatar')) {
$media = Asset::image()->saveFromFormUpload(new Media(), Input::file('avatar'));
$user->avatar = $media->id;
}
$user->role_id = Input::get('role');
$user->first_name = Input::get('first_name');
$user->last_name = Input::get('last_name');
$user->email = Input::get('email');
$user->username = Input::get('username');
$user->password = Hash::make(Input::get('password'));
$user->address = Input::get('address');
$user->postcode = Input::get('postcode');
$user->suburb = Input::get('suburb');
$user->state = Input::get('state');
$user->country_id = Input::get('country');
$user->status = Input::get('status');
$user->save();
return Redirect::route('admin.users')
->with('message', 'You have successfully created a new User.');
}
}
This might help. I didn't test it since I don't actually have access to laravel setup now.
A simple service can be used in this case..IMO
class addNewUserService
{
private $rules = [
'avatar' => array('image', 'mimes:jpeg,png,gif'),
'role' => array('required', 'exists:role,id'),
'username' => array('required', 'min:3', 'max:30', 'unique:user'),
'email' => array('required', 'email', 'unique:user'),
'first_name' => array('required', 'min:3', 'max:100'),
'last_name' => array('required', 'min:3', 'max:100'),
'password' => array('required', 'min:8', 'confirmed'),
'address' => array('required', 'min:3', 'max:100'),
'postcode' => array('required', 'integer'),
'suburb' => array('required', 'min:3', 'max:30'),
'state' => array('required', 'min:3', 'max:30'),
'country' => array('required', 'exists:country,id'),
'status' => array('required', 'in:'.implode(",", array_keys(User::getStatusTypes())))
];
private $userRepository;
private $validator;
public function __construct(UserRepositoryInterface $userRepository, Validator $validator)
{
$this->userRepository = $postRepository;
$this->validator = $validator;
}
public function add(array $data)
{
$validator = $this->validator->make($data, $this->rules);
if ($validator->fails()) {
throw new anExceptionClassGoesHere($validator->errors());
}
return $this->userRepository->create($data);
}
}
class aController
{
private $addNewUserService;
public function __construct(addNewUserService $addNewUserService)
{
$this->addNewUserService = $addNewUserService;
}
public function postAdd()
{
try {
$this->addNewUserService->add(Input::all());
return Redirect::route('admin.users')->with('message', 'a message');
} catch (anExceptionClassGoesHere $e) {
// the exception class should accept a validator and provide 'getErrors' getter
return Redirect::route('admin.users.add')
->withErrors($e->getErrors())
->withInput(Input::except('password'));
}
}
}
As @thepsion5 pointed out, there're a few places where you can validate your inputs. The example validates inputs in the application service level.
@moon0326 is that Service a Laravel thing (I have not really seen it used) or is it just a standard pattern?
Do you know where I can read up on it and work out what I can actually add into it?
Also in that instance a Service is doing the validation and creation of the user. Should the creation of the user still be in the controller so its partially decoupled?
it's not Laravel specific thing. An idea of application/domain service is widely used in domain driven design and other software designs. Since many of Laravers borrow ideas from DDD, it is probably best to use what DDD might use. Please don't take my class as an answer since I'm still learning DDD as well.
If you create a user in your controller, you actually have to copy and paste the code in your controller to other controllers if you want to use the same functionality. Separating the logic out of the controller into a service makes it more flexible IMO. I know that it's such a simple operation. I think you can do it in your controller, but it's really up to you.
I've been reading the following articles/books (adding..)
-
Domain-Driven Design (people call it bluebook) - http://www.amazon.com/Domain-Driven-Design-Tackling-Complexity-Software/dp/0321125215/ref=sr_1_1?ie=UTF8&qid=1398990167&sr=8-1&keywords=domain+driven+design
-
Implementing Domain-Driven Design (http://www.amazon.com/Implementing-Domain-Driven-Design-Vaughn-Vernon/dp/0321834577/ref=sr_1_2?ie=UTF8&qid=1398990260&sr=8-2&keywords=domain+driven+design)
-
POJOs in Action (http://www.amazon.com/POJOs-Action-Developing-Applications-Lightweight/dp/1932394583/ref=sr_1_1?ie=UTF8&qid=1398990285&sr=8-1&keywords=pojo+in+action)
-
Improving Applications Design with a rich Domain Model (http://www.parleys.com/play/514892250364bc17fc56bb63/chapter0/about)
-
DDD sample of Eric Evan's Cargo application (https://github.com/codeliner/php-ddd-cargo-sample) in PHP
-
DDD PHP forum (https://groups.google.com/forum/#!forum/dddinphp)
There're just too many links. I can add all of them, but the above materials will get you started.
Let me know if you need more links.
If you're new to these repositories, services, and other patterns, Don't worry about being wrong. Just make things out and refactor them as you go. These DDD related stuff might be too much if you're not familiar with other patterns.
Hope it helps.
Ok I have had an attempt at just trying to make some aspects of my code cleaner and to start using Repository/Services although Services for me is just Validator. Would love to get some input from you all.
UserController.php
<?php
namespace Fdw\Core\Controllers\Admin;
use Auth;
use Input;
use Redirect;
use View;
use Fdw\Core\Models\User;
use Fdw\Core\Repositories\UserRepository;
use Fdw\Core\Validators\UserValidator;
use Fdw\Core\Validators\ValidatorException;
class UserController extends BaseController {
private $repository;
private $validator;
public function __construct(UserRepository $repository, UserValidator $validator)
{
$this->repository = $repository;
$this->validator = $validator;
}
public function getLogin()
{
return View::make('core::admin.user.login');
}
public function postLogin()
{
try {
$validator = $this->validator->login(Input::all());
if (!Auth::attempt(array_add(Input::except('_token'), 'status', User::STATUS_ACTIVE))) {
throw new ValidatorException(ValidatorException::error('Either your email or password is incorrect.'));
}
return Redirect::route('fdw.admin.dashboard')
->with('message', 'You have successfully logged in.');
} catch (ValidatorException $e) {
return Redirect::route('fdw.admin.user.login')
->withErrors($e->errors())
->withInput(Input::except('password'));
}
}
}
UserValidator.php - Needs a bit of work. Not sure about how I am doing the rules.
<?php
namespace Fdw\Core\Validators;
use Input;
use Fdw\Core\Models\User;
use Fdw\Core\Repositories\UserRepository;
class UserValidator extends BaseValidator implements ValidatorInterface {
public function rules()
{
return array(
'avatar' => array('image', 'mimes:jpeg,png,gif'),
'role' => array('required', 'exists:role,id'),
'username' => array('required', 'min:3', 'max:30', 'unique:user'),
'email' => array('required', 'email'/*, 'unique:user'*/),
'first_name' => array('required', 'min:3', 'max:100'),
'last_name' => array('required', 'min:3', 'max:100'),
'password' => array('required', 'min:8'/*, 'confirmed'*/),
'address' => array('required', 'min:3', 'max:100'),
'postcode' => array('required', 'integer'),
'suburb' => array('required', 'min:3', 'max:30'),
'state' => array('required', 'min:3', 'max:30'),
'country' => array('required', 'exists:country,id'),
'status' => array('required', 'in:'.implode(",", array_keys(User::getStatusTypes())))
);
}
public function login(array $data)
{
$rules = array_only($this->rules(), array('email', 'password'));
$this->validate($data, $rules);
}
public function add(array $data)
{
$rules = array_only($this->rules(), array(
'avatar',
'role',
'username',
'email',
'first_name',
'last_name',
'password',
'address',
'postcode',
'suburb',
'state',
'country',
'status'
));
$this->validate($data, $rules);
}
}
BaseValidator.php
<?php
namespace Fdw\Core\Validators;
use Validator;
class BaseValidator {
public function validate($data, $rules) {
$validator = Validator::make($data, $rules);
if ($validator->fails()) {
throw new ValidatorException($validator->errors());
}
}
}
ValidatorException.php
<?php
namespace Fdw\Core\Validators;
use Exception;
class ValidatorException extends Exception {
public function errors()
{
return json_decode($this->getMessage());
}
public static function error($error)
{
return json_encode(array('error' => $error));
}
}
Also I would LOVE to get input on my current folder structure :)
For me a repository is only an interface between some sort of persistence layer. So anything outside giving it items to store or taking items out is best left outside of the repository.
To overcome the chance of parsing invalid data to our persistence layer a solution is to only accept entity popos (plain old php objects) instead of associative arrays or stdClass objects. The php object should not allow any invalid values to set on it removing the need for validation in the repository.
This presents another problem though. How do we return the correct collections of entities. The solution is to map all results from the repository to an entityFactory.
In my current application all my repositories return a laravel Collection of entities (e.g a Collection of Users where User is just a popo).
This solution takes longer but it helps to remove any non OO code from my system. I really dislike using associative arrays to hold data for a domain object.
- I would have two separate validator classes.
- I think Auth::attempts() takes care of validating your password. Do you need a separate validator for it?
- I would not use 'add' for the method name in the validator. A few ideas on validating inputs.
http://tntstudio.us/blog/laravel-form-validation-as-a-service
http://culttt.com/2014/01/13/advanced-validation-service-laravel-4/
https://github.com/laracasts/Form-Validation-Simplified (take a look at this one. It's pretty simple and good)
Unless you want to use services, I think your method is simplest way to go..given your code base.
Sign in to participate in this thread!
Laravel
Laravel News
Laracasts
Laravel Podcast