Laravel по-русски
Laravel по-русски
Русское сообщество разработки на PHP-фреймворке Laravel.
Ты не вошёл. Вход тут.
Добрый день! Учусь ларавель, делаем тестовый проект типа облачного хостинга файлов, который возможно станет реальным.
На данный момент все в стадии развернули ларавель, система авторизации (ларавельная) с активацией и сменой мейла (своими), также загрузка и вывод аватара (свои на основе Mediable). Далее в планах подключения биллинга и работа с файлами пользователей.
Все, что сейчас сделано работает. Но хотелось бы советов, по оптимальному проектированию классов.
Для начала примерно показываю что сделал.
Для авторизации и смены мейла написал похожие друг на друга сервисы. И сделал для них интерфейсы и зарегил сервис провайдер. Они включают в себя методы для обработки как запроса на отсылку письма с подтвержением регистрации/нового мейла, так и обработку клика по ссылке из данных писем.
Токены подтвержения регистрации и смены мейла и сам новый мейл, храню в двух отдельных табличках, не в users. И для каждой этой табличке по модели элоквент и по репозиторию.
Привожу код для случая смены мейла:
Route::get('home/account/email', ['middleware' => ['auth', 'isVerified'], 'uses' => 'Auth\ChangeEmailController@showForm'])->name('home.account.email');
Route::post('home/account/email_save', ['middleware' => ['auth', 'isVerified'], 'uses' => 'Auth\ChangeEmailController@saveForm'])->name('home.account.email_save');
Route::get('home/account/email_set/{token}', ['middleware' => ['auth', 'isVerified'], 'uses' => 'Auth\ChangeEmailController@emailSet'])->name('home.account.email_set');
Методы контроллера запроса на смену и смены по ссылке из письма
public function saveForm(Request $request, ChangeEmailContract $changeEmailService)
{
$user = Auth::user();
$rules = [
'email' => 'required|email|unique:users',
'password' => 'required|checkpassword:'.$user->email,
];
$messages = [
'email.required' => 'Please enter an email address',
'email.email' => 'Please enter a valid email address',
'email.unique' => 'This e-mail is already taken. ',
'password.required' => 'Please enter your password',
'password.checkpassword' => 'Your enter wrong password',
];
Validator::make($request::all(), $rules, $messages)->validate();
$changeEmailService->sendChangeEmailMail($user, Request::get('email'));
return redirect()->route('home')->with('status', "Confirmation change E-mail link send to ".Request::get('email'));
}
public function emailSet($token, ChangeEmailContract $changeEmailService)
{
$email = Request::get('email');
try {
$user = $changeEmailService->setEmail($token, $email);
}
catch (\App\Exceptions\ChangeEmailNotFoundException $e) {
return redirect()->route('home')
->with('status', $e->getMessage());
}
Auth::login($user);
return redirect()->route('home')
->with('status', 'You successfully activated your new email!');
}
namespace App\Services\Auth;
use \App\Models\User;
use \App\Contracts\Auth\ChangeEmailContract;
use \App\Contracts\Auth\ChangeEmailRepositoryContract;
use \App\Exceptions\ChangeEmailNotFoundException;
use Illuminate\Mail\Mailer;
use Illuminate\Mail\Message;
class ChangeEmailService implements ChangeEmailContract
{
protected $mailer;
protected $changeEmailRepo;
public function __construct(ChangeEmailRepositoryContract $changeEmailRepo)
{
$this->changeEmailRepo = $changeEmailRepo;
}
public function sendChangeEmailMail($user, $email)
{
$token = $this->changeEmailRepo->createEmailChange($user, $email);
\Mail::to($email)->send(
new \App\Mail\ChangeEmail(array(
'email' => $email,
'token' => $token,
))
);
}
public function setEmail($token, $email)
{
$changeEmail = $this->changeEmailRepo->getChangeEmailByTokenAndEmail($token, $email);
if ($changeEmail === null) {
throw new ChangeEmailNotFoundException();
}
$user = User::find($changeEmail->user_id);
if (!$user) {
throw new ChangeEmailNotFoundException();
}
$user->email = $email;
$user->save();
$this->changeEmailRepo->deleteChangeEmail($token);
return $user;
}
}
Репо вокруг таблицы где токены и новые мейлы. Понимаю что модель нужно было включить иньекцией, но я предпочел просто обращаться к ней через ORM методы.
namespace App\Repositories\Auth;
use \App\Contracts\Auth\ChangeEmailRepositoryContract;
use \App\Models\EmailChange;
use Carbon\Carbon;
use Illuminate\Database\Connection;
use Illuminate\Support\Facades\DB;
class ChangeEmailRepository implements ChangeEmailRepositoryContract
{
public function createEmailChange($user, $email)
{
$email_change = $this->getEmailChange($user);
if (!$email_change) {
return $this->createEmailChangeRecord($user, $email);
}
return $this->updateEmailChangeRecord($user, $email);
}
public function getEmailChange($user)
{
return EmailChange::where('user_id', $user->id)->first();
}
public function getChangeEmailByTokenAndEmail($token, $email)
{
return EmailChange::where(array('token' => $token, 'email' => $email))->first();
}
public function deleteChangeEmail($token)
{
EmailChange::where('token', $token)->delete();
}
private function updateEmailChangeRecord($user, $email)
{
$token = $this->getToken();
EmailChange::where('user_id', $user->id)->update([
'token' => $token,
'email' => $email,
'created_at' => new Carbon()
]);
return $token;
}
private function getToken()
{
return hash_hmac('sha256', str_random(40), config('app.key'));
}
private function createEmailChangeRecord($user, $email)
{
$token = $this->getToken();
EmailChange::insert([
'user_id' => $user->id,
'token' => $token,
'email' => $email,
'created_at' => new Carbon()
]);
return $token;
}
}
namespace App\Models;
use Illuminate\Database\Eloquent\Model;
class EmailChange extends Model
{
protected $table = 'email_change';
public function user()
{
return $this->belongsTo(User::class);
}
}
Моделька юзера. Метод отправки письма, связан не с данным, функционалом, а со сменой пароля, которая встроенная используется.
namespace App\Models;
use Illuminate\Notifications\Notifiable;
use App\Notifications\CustomResetPassword;
use Illuminate\Foundation\Auth\User as Authenticatable;
use \Plunk\Mediable;
class User extends Authenticatable
{
use Notifiable;
use \Plank\Mediable\Mediable;
protected $fillable = [
'name', 'email', 'password',
];
protected $hidden = [
'password', 'remember_token',
];
public function sendPasswordResetNotification($token)
{
$this->notify(new CustomResetPassword($token));
}
}
Сервис провайдеры для сервиса и репозитория
namespace App\Providers\Auth;
use Illuminate\Support\ServiceProvider;
class ChangeEmailProvider extends ServiceProvider
{
protected $defer = false;
public function register()
{
$this->app->bind('App\Contracts\Auth\ChangeEmailContract', function ($app) {
return new \App\Services\Auth\ChangeEmailService(
$app -> make("\App\Contracts\Auth\ChangeEmailRepositoryContract")
);
});
}
public function provides()
{
return ['\App\Contracts\Auth\ChangeEmailContract'];
}
public function boot()
{
}
}
namespace App\Providers\Auth;
use Illuminate\Support\ServiceProvider;
class ChangeEmailRepositoryProvider extends ServiceProvider
{
protected $defer = false;
public function register()
{
$this->app->bind('\App\Contracts\Auth\ChangeEmailRepositoryContract', function ($app) {
return new \App\Repositories\Auth\ChangeEmailRepository();
});
}
public function provides()
{
return ['\App\Contracts\Auth\ChangeEmailRepositoryContract'];
}
public function boot()
{
}
}
Как я хочу все это отрефакторить?
Я думаю нужно создать UserRepository, его явно не хватает. В него конечно нужно будет добавить саму регистрацию, смену аватара, а из данного функционала, пожалуй те небольшие процедуры которые относятся все же именно к таблице users например постановка, статус активирован, установка нового мейла…
Тогда получится для каждой таблицы свой репозиторий. В общем то логично. Но кажется более практично было бы, так как активация и смена мейла, в общем то также тесно связанные именно с юзеров вещи и все обращения к таблицам ChangeEmail и Activation а так же генерацию и проверку токена перенести в UserRepository. Да он будет толще зато смогу убрать два репозитория ChangeEmailRepository и ActivateEmailRepository и также два контракта и два сервис провайдера…
Тогда получится юзеру и связанным тесно с ними табличкам общий репозиторий, а вот сервисы отдельные для смены емайла, активации и аватара оставить.
Как считаете, так было бы оптимальнее?
Так же прошу подсказать есть в моем уже существующем и приведенном коде какие то явные ошибки?
И еще вопрос. А даже если ChangeEmailRepository и ActivateEmailRepository оставить отдельными и не переносить их содержимое в UserRepository, то нужны ли для этих репо контракты и сервис провайдеры, учитывая что подменять реализацию я ведь вряд ли буду…
Не в сети
Сильно не вникал, но у тебя на простейшую задачу куча классов (репозиторий, сервис провайдер, интерфейс, сервис). Ты можешь в две строчки это все запилить в контроллере и в 2-3 однострочных метода в модели, используя Eloquent и notifications. Плюс использование репозитория с моделями Eloquent - это оверинжениринг, абсолютно ненужная абстракция, которую все новички суют в свои проекты. И еще, группируй маршруты по посредникам (middleware) и почисти контроллер: валидацию перенеси в Request класс, кастомные сообщения в языковые файлы.
Изменено AlexeyMezenin (06.08.2017 17:55:45)
Не в сети
Спасибо за мысль. Я тоже считаю что лишнего много.
В тоже время хочется и правильности и универсальности поэтому до предела упрощать не хочется.
- 2-3 однострочных метода в модели,
Прямо в классах Eloquent не хочется. Кажется репо все же нужно. туда же можно для работы с юзером и много другое пихнуть. Кроме того например коллекцию из нескольких юзеров лучше в репо получать. Это более логично чем в модельке eloquent. Ну и потом а если на Doctrine переметнуться? А такая мысль есть…
В общем на eloqent не хочется затачиваться… И Repo будет.
А вот от контракта и от провайдера, для репо можно без потерь отказаться? И также имеет ли смысл в Repo для юзера запихнуть его авторизацию и смену мыла? Вот эти вопросы кажется ключевые. Упростив именно все это кажется будет уже и достаточно…
Про отказ от сервисов и перенос в контроллер, тут + на —, думаю можно и так и сяк…
Остальные замечания принимаю. Языковых файлов пока не надо. Сайт только на english
Не в сети
Кстати если я оставляю все таки сервисы. То видимо лучше как и сейчас регистрировать их каждый в своем сервис провайдере? тк каждый из них используется лишь на одной двух страницах сайта… И нужно включить отложенную загрузку… Хотя честно не понимаю какой прирост в скорости от того что простенький класс не создастся… А если сервис используется много где, то лучше в AppService для всего сайта?
А репозиторий UserRepository видимо уж точно лучше биндить в AppService? Он то для всего нужен…
И все это лучше именно bind, а не singleton как понимаю… Тк ничего такого что нужно было бы прохождении запроса сохранять между вызовами методов класса, в его экземпляре нет. А значит нет надобности что бы этот экземпляр был единственным…
Не в сети
Не в сети
Ты все выслушал и делаешь по-своему, зачем вопрос тогда задавал?
На доктрину ты не метнешься. Не видел ни одного Laravel проекта, в котором использование репозитория давало какой-то плюс и ни разу не видел метнувшихся с Eloquent на Doctrine в Laravel проекте. А вот гемора от этих пакетов море, StackOverflow и подобные сайты завалены вопросами новичков в духе "я использую репозиторий, у меня из-за этого тут вот это не работает...". Ты прочитал о репозиториях и бездумно используешь их в проекте. Репозитории хороши когда ты используешь сырые запросы или обращения к стороннему API, например.
Языковые файлы. Здесь не идет речь о Russian/English. Кастомные сообщения хранятся в языковых файлах. Они должны лежать в одном месте или в Request классе, на худой конец, а не разбросаны по всему приложению. А вообще, это хорошая практика - хранить любой текст в языковых файлах, даже в моноязычных приложениях.
Контроллеры. Читай про задачу контроллеров в MVC и про fat model, skinny controller.
Авторизация. Если используешь встроенную авторизацию, то там вообще ничего не нужно делать, все уже есть. Если пишешь свою, то задай себе вопрос "зачем?" и используй встроенную.
Ты можешь изобретать велосипеды и использовать их, если твоя цель "лишь бы работало". Если цель звучит как "стать хорошим разработчиком" и/или "писать поддерживаемый код", тогда читай документацию и книги/статьи о хороших практиках при разработке.
Изменено AlexeyMezenin (06.08.2017 18:48:59)
Не в сети
Понимаю, что все зависит от задачи… Но вот например вывод аватара… допустим он нужен наверху авторизованному юзеру в хидере, нужен где нибудь на отдельной странице где скажем список юзеров онлайн, список друзей юзера, или тому подобное, те список юзеров и нужен на странице его загрузки и обновления возможно более крупная версия…
Брать ссылку на аватар, которую сформировали в вью-композере для хидера, хранить ее внутри объекта-сервиса, что бы вывести где то еще скорее не нужно. Потому синглтон не нужен.
Но и повторять код формирования ссылки на аватар в вью-композере для хидере, затем в контроллере где текущий аватар для обновления или где список юзеров также не нужно.
Значит выход делаем просто bind класса-сервиса. И обращаемся в вью-композере и указанных контроллерах, либо чем экземпляр класса полученный иньекцией зависимости или через фасад…
Таким образом и код не повторяется и лишних сущностей или хранения каких то значений не нужных в контексте всего приложения нет.
Не в сети
Не в сети
«Репозитории хороши когда ты используешь сырые запросы или обращения к стороннему API»
Сырые запросы будут. Сайт не простой, одним элоквент не обойтись скорее… Возможно и обращения к стороннему API будут например к биллингу или к API сервера где файлы хранятся.
Кроме того мы хотим использовать лару на старом и довольно сложном самописном сайте сначала сделав на ней админку к нему. Там уже существующие таблицы забитые милионами записей… И миграции с нуля не начнешь накатывать…
С учетом сказанного использование Repo хотя бы только и именно для User и возможно проба внедрить доктрину пусть не в этом, так в админке, оправданно??
Не в сети
Не в сети
Не в сети
« тогда читай документацию и книги/статьи о хороших практиках при разработке.»
Я читал все что нашел релевантное своей задачи. И вопрос мой тоже из-за этого. Так как есть работает. И если просто взять и юзать лишь элоквент из контроллеров тоже будет работать…
Я хочу нечто среднее и с прицелом под задачи о которых выше пишу.
Не в сети
Не в сети
Но вот например вывод аватара… допустим он нужен наверху авторизованному юзеру
Смотря где ты хранишь ссылку на аватарку (S3, локально и пр.). Обычно это простейший код в представлении, что-то вроде:
Мастер
Писарь<img src="{{ asset('avatars/'.auth()->id().'.png')) }}">Когда аватарки в S3 или подобном хранилище, код примерно такой же с парой нюансов.
список юзеров онлайн, список друзей юзера
Как я понял, тебе список нужен на одной или двух страницах, тогда почему бы не передать список в контроллере:
return view('my_view', [
'topUsers' => $this->user->getTopUsers(),
'friends' => $this->user->getTopFriends()
]);Это простые задачи, которые незачем усложнять.
Не в сети
Насчет аватара. Код его получения примерно такой:
$user = Auth::user();
if ($user) {
if($user->hasMedia(’avatar’)) {
$user_avatar_url = $user->getMedia(’avatar’)->first()->getUrl();
}
}
Тк я использую пакет mediable. Хранить просто как user_id.jpg не хочу. Тк с аватаром может пока и все просто, но вообще нужно хранить оригиналы несколько версий ресайза в разный размер, что бы юзер вырезал показываемую область, галлерии изображений.
В общем в перспективе то нам нужно проект enterpise уровня на laravel сделать. Тк есть https://www.realmusic.ru/ с ужасным кодом старым, нужно или его переписать или заново что то на основе тех наработок… В общем нужно заложить фундамент по универсальней…
Изменено htclog81 (06.08.2017 19:18:07)
Не в сети
Как я понял, тебе список нужен на одной или двух страницах, тогда почему бы не передать список в контроллере:return view('my_view', [
'topUsers' => $this->user->getTopUsers(),
'friends' => $this->user->getTopFriends()
]);Это простые задачи, которые незачем усложнять.
Да, я все это понимаю. Но нет с картинками чуть сложнее и пакет нам понравился. Впрочем сейчас актуальнее самая основа про активацию и смену мейла, про UserRepositiry и сервисы и как все это верно биндить. В общем самая основа проекта.
Выше писал. Ответишь?
Не в сети
Я так и хочу. Вроде бы особо бизнес логики в контроллере нет.
Ну у тебя в методе контроллера 15 строк вместо 2. Это не skinny controller.
Но если какая то процедура с использованием модели делается лишь в одном и заведомо одном контроллере. То прямо в контроллере и пишем обращения к модели или к репо и не морочимся сервисом?
В обучалках запрос пишут прямо в контроллере. Для обучения это нормально, но для реального приложения необходимо убрать из контроллера сами запросы. Контроллеры в MVC служат для связки модели и представления. Пример я привел выше. Запрос хранится в модели (репозитории).
Хорошо ты против репозитория это я понял. А сервиса?
Твой сервис выглядит как пара методов, которые я бы разбросал в модели и контроллер. Плюс такого решения - нет лишней абстракции в виде сервиса, поэтому код с ходу прочитает любой разработчик. Плюсов в использовании сервиса в том виде, в котором ты его используешь, я не вижу.
Не в сети
Итого сервис не нужен, репо нужен.
В репо убираем все запросы к БД. и дергаем репо из контроллера. Согласен, что в случае смены мейла, в сервисе мало чего для повторного пользования…
Но в случае если все таки одни и теже запросы к репо с одной и той же логикой используется в нескольких контроллерах, то тогда нужно вынести в сервиc??
Изменено htclog81 (06.08.2017 19:30:38)
Не в сети
Выше писал. Ответишь?
Ты вопросы клепаешь быстрее, чем я ответы пишу. 
Насчет аватара. Код его получения примерно такой
А почему бы не заменить код, который ты показал, этим кодом в представлении:
@if (auth()->check() && auth()->user()->hasMedia('avatar'))
<img src="{{ auth()->user()->getMedia(’avatar’)->first()->getUrl() }}">
@endifЗдесь нужно следить, чтобы методы hasMedia и getMedia не наделали дополнительных запросов в БД, особенно если используются сторонние пакеты.
Изменено AlexeyMezenin (06.08.2017 19:32:30)
Не в сети
Не в сети
Выше писал. Ответишь?Ты вопросы клепаешь быстрее, чем я ответы пишу.
я же волнуюсь скоро отпуск через несколько дней, а я хочу оставить годную основу для сайта
Не в сети
Итого сервис не нужен, репо нужен.Репо тоже не нужен. 
Не в сети
...
Здесь нужно следить, чтобы методы hasMedia и getMedia не наделали дополнительных запросов в БД, особенно если используются сторонние пакеты.
Проверю. Скорее дают.. Но как иначе я узнаю показать ли аву.. Впрочем это сейчас не главные. Все равно это запросы по ключу выбирающие одну строку. И десятков миллионов юзеров пока не придвидется
Изменено htclog81 (06.08.2017 19:36:17)
Не в сети
- Repo не нужен
Где ты предлагаешь бизнес логику для юзера писать прямо в модельке элокьент? В классе Models\User?
Мне кажется это неверно… Это же класс именно Eloquent зачем я буду писать в него скажем метод отдающий на выходе скажем коллекцию юзеров… Это же совершенно не логично. Класс описывает одного юзера причем ради использования его в контексте запросов элокьент, а я возвращаю коллекцию…
Или ты об том, что бы вообще никуда не собирать бизнес-логику и запросы для юзера. А прямо в контроллере писать Eloquent запросы? Но не хочется это по контроллерам разбрасывать…
Изменено htclog81 (06.08.2017 19:39:56)
Не в сети
А методы работы с будующим функционалом связанным с юзером, например файлы юзера (не авы), подписки и платежи юзера? Это что тоже в eloquent моделях? Каких Models\User неверно кажется да и гигантская будет. В Models\User и Models\UserFiles например? Тоже кажется не очень тк совмещаем модель как отражение таблицы конкретной которых может быть несколько для функционала сайта и модель как место хранения бизнес логики — юзеров, подписок юзеров, файлов юзеров итд.
Разбрасывать элоквент запросы по контроллерам и вью композерам тоже не хочется… Ну пусть не доктрине, пусть ее не будет. Но все таки слой где все что связанно с запросами весьма удобен в большом приложении…
Изменено htclog81 (06.08.2017 19:43:51)
Не в сети
Универсальная вики-разметка
Работает на FluxBB (перевод Laravel.ru)