In my app users can create and soft-delete payment gateways something like Paypal. Users of a website can connect to the gateway and make payments.
I've written the logic in a transaction also locking a matching record (on website
field) which was soft-deleted by the user because of race-condition. Read code for a better understanding.
Already exists (soft-deleted)
These gateways must be verified by admin, if the user submits different website
and description
fields in comparison to the matching record which was already verified, set is_verified
and rejection_reason
to null
is_verified
values:
true
verifiedfalse
rejectednull
pending
The Rule: Create if not exists, Restore if soft-deleted.
Somehow i think the controller can be simplified and refactored. Performance and Consistency both are important for me.
Migration
Schema::create('gateways', function (Blueprint $table) {
$table->id();
$table->foreignId('user_id')
->constrained()
->onUpdate('cascade')
->onDelete('cascade');
$table->string('title', 50);
$table->text('description');
$table->string('website')->unique();
$table->enum('wage', Gateway::getWageKeys());
$table->text('rejection_reason')->nullable();
$table->boolean('is_verified')->nullable()->default(null);
$table->boolean('is_blocked')->default(false);
$table->boolean('first_payment')->default(false);
$table->dateTime('free_until')->nullable();
$table->softDeletes();
$table->timestamps();
});
Controller
/**
* Create a new gateway.
*
* @param \App\Http\Requests\StoreGatewayRequest $request
* @return \App\Http\Resources\GatewayResource
*/
public function create(StoreGatewayRequest $request)
{
try {
$gateway = DB::transaction(function () use ($request) {
$gateway = $request->user()->gateways()->onlyTrashed()
->where('website', $request->website)->lockForUpdate()->first();
if (! $gateway) {
return $request->user()->gateways()->create($request->validated());
} else {
if ($gateway->is_blocked)
abort(452);
$gateway->fill($request->validated());
if ($gateway->isDirty('website') || $gateway->isDirty('description')) {
$gateway->rejection_reason = null;
$gateway->is_verified = null;
}
$gateway->deleted_at = null;
$gateway->save();
return $gateway;
}
});
} catch (QueryException $e) {
if ($e->errorInfo[1] === 1062) {
abort(409);
} else {
throw $e;
}
}
return new GatewayResource($gateway);
}
1 Answer 1
- I'd personally suggest separating a logic into different files.
i.e. Controller would look something like below if you remove the business logic.
public function create(StoreGatewayRequest $request)
{
$gateway = app(IGatewayCreator::class)->handle($request->validated, $request->user());
return new GatewayResource($gateway);
}
This way within controller class is easier to understand and test.
The next part that we have here is related to the exception handling. We can leverage Laravel's Exception handler to handle this for us - https://laravel.com/docs/8.x/errors
Within your logic we have two concrete strategies, one for when we create a new record and another for when we handle soft deleted records. Here we can separate concerns further.
class GatewayCreator implements IGatewayCreator
{
// i've used $dto here instead of an array,
// since with DTO you can check what attributes are within the DTO
public function handle(GatewayDTO $dto, User $user): Gateway
{
$strategy = $this->strategy($dto->website, $user);
return app($strategy)->handle($dto, $user);
}
protected function strategy(string $website, User $user): string
{
$recordExists = $user->gateways()
->onlyTrashed()
->hasWebsite($website) // local scope
->exists();
return $recordExists
? IStoreSoftDeletedGateway::class
: IStoreNewGateway::class;
}
}
The strategy for the
IStoreNewGateway
is self-explanatory, just use a store within database transactionsThe strategy for
IStoreSoftDeletedGateway
will have other logic.
public function handle(GatewayDTO $dto, User $user): Gateway
{
$gateway = $user->gateways()
->onlyTrashed()
->hasWebsite($dto->website)
->lockForUpdate()
->first();
if ($gateway->is_blocked) {
throw new GatewayRecordIsLockedException();
}
return DB::transaction(function () use (Gateway $gateway, GatewayDTO $dto) {
// for unit testing you can probably extract lines below to a different method.
$gateway->fill($dto->toArray());
// just set rejection_reason and is_verified to null without checking for dirty
$gateway->resetRecord();
$gateway->deleted_at = null;
$gateway->save();
return $gateway;
});
}
NOTES
- I don't understand why you have
lockForUpdate
- It's not clear why you had race conditions, shouldn't this be something to deal with on FE? You can also consider queueing requests.
- It's also not clear what performance issues you've experienced.