Refactor, Tiny Enhancements and Bug Fixes

[MohannadNaj]

Hello @mpociot

Thanks for this package, Smart and neat! .. Used it in one project where it allowed me to fix N+1 issues in bulk throughout the code base, a real time saver. Thank you!

---

Without a breaking change, This PR includes the following:

- Basic code refactor

Targeting src/QueryDetector.php, Using Concerns traits, Return early if statements.

Commits:

5f2dfd686b5da2691e174e20df488abe5b50b005 Refactor to Return Early if statements 033258340db5d3eb4d4170a8ea1a38405d899114 Refactor to HasContext Concern 11bc17b6710ade8b186643c879fd0a92f8a2f41c Refactor to InteractsWithSourceFiles Concern 946d1ea3ad94d8ee911d1cd9cfce0acb71e57e31 Refactor to Bootable Concern

---

- Adaptability to multiple-requests contexts

Possibly closes: #70 Possibly closes: #71

Problem:

What I mean by multiple requests contexts, when we do multiple requests/responses lifecycles within one Laravel container app() instance.

Commonly this happens in Feature/Integration tests. Where we might do multiple requests inside one test.

To even simplify the problem, We already have in our unit tests this passing test:

    /** @test */
    public function it_does_not_fire_an_event_if_there_is_no_n1_query()
    {
        Event::fake();

        Route::get('/', function (){
            $authors = Author::with('profile')->get();

            foreach ($authors as $author) {
                $author->profile;
            }
        });

        $this->get('/');

        Event::assertNotDispatched(QueryDetected::class);
    }

If we just duplicated the visit $this->get('/'); $this->get('/');, it will fail and it will claim we are having N+1 issue.

Solution:

This was solved by adding a $context variable to be part of the calculated query key. So the key now includes the context variable, and we will automatically register a new random string context after each request.

The key will be (In src/QueryDetector.php) :

$key = md5($this->context . $query->sql . $model . $relationName . $sources[0]->name . $sources[0]->line);

And in the middleware we will register a new context after each request. (In src/QueryDetectorMiddleware.php) :

        /** @var \Illuminate\Http\Response $response */
        $response = $next($request);

        $this->detector->newContext();

        // Modify the response to add the Debugbar
        $this->detector->output($request, $response);

Now, a userland code for isolating queries in dispatched jobs might look something like this:

        Event::listen(function (JobProcessing $jobProcessingEvent) {
            app('querydetector')->newContext();
        });

Commits: a691bc2f801cacf530c213e8d9222808d1bac900 Always clear context after requests 7e370ddbb3fc0bf2d38dad876f0e6fc33becd8d8 Add Contexts Feature

---

- Other Bug Fixes and Enhancements

While working on this context feature, I had to solve some older issues.

1. Bug: Booting multiple times.

The code in the middleware before executing the request:

$this->detector->boot();

Now it's

$this->detector->bootIfNotBooted();

Since booting multiple times means registering multiple database listeners DB::listen since it's in the boot function.

Commit: 0f692b58c066b52fd308a37dac94e688246c2198 Safely boot the service in controllable and manageable way

2. Dispatching multiple events

The code in BeyondCode\QueryDetector\QueryDetector::output($request, $response) function, which get called from the middleware, will call the function getDetectedQueries multiple times.

This function, BeyondCode\QueryDetector\QueryDetector::getDetectedQueries, encapsulates dispatching the event QueryDetected.

The solution was to call getDetectedQueries once and pass it's results around to the needed methods.

I considered extracting the event dispatch logic from getDetectedQueries, to prevent a "getSomething" method from performing any actions silently, but then I thought that should be for further development in a separate PR.

Commit: a3ca8f2de87540895dd7792c52adb7e9b5b48f41 Fix dispatching multiple events