Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add getter and setter for connection in the DatabaseBatchRepository class #43869

Merged
merged 2 commits into from
Aug 26, 2022

Conversation

abrardev99
Copy link
Contributor

This simple PR adds setConnection and getConnection methods in the DatabaseBatchRepository class.

Why

In the tenancy package, We are adding support for Laravel job batches which required temporarily replacing the BatchRepository connection with the correct tenant connection and then reverting it back when exiting the tenant context.

The current implementation uses the PHP Reflection class. It would be nice to have these methods in the framework to get rid of the Reflection boilerplate and modify the connection easily.

Wondering how our code would look:

Before

$batchRepositoryReflection = new ReflectionClass($batchRepository);
$connectionProperty        = $batchRepositoryReflection->getProperty('connection');
$connectionProperty->setAccessible(true);
$connection = $connectionProperty->getValue($batchRepository);

$this->previousConnection = $connection;

$connectionProperty->setValue($batchRepository, DB::connection('tenant'));

After

$this->previousConnection = $batchRepository->getConnection();
$batchRepository->setConnection(DB::connection('tenant'));

Tests

I haven't added tests because, as I said, these are simple changes, not a feature. I'm not sure if we need tests for getters and setters. If still wants me to add tests, please comment, and I'll add them.

@taylorotwell taylorotwell merged commit 2a1a55c into laravel:9.x Aug 26, 2022
@abrardev99 abrardev99 deleted the connection-getter-setter branch August 28, 2022 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants