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

[Feat] : Implement caching mechanism for collection queries #327

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

mohaphez
Copy link
Contributor

Implemented caching for collection queries to reduce redundant database calls and improve performance.
Cached responses for 30 seconds based on unique query parameters.

Copy link

what-the-diff bot commented Sep 12, 2024

PR Summary

  • Improvement in Data Handling
    The code now supports both new and old structures of the dataSource variable, ensuring a more flexible method of handling data.

  • Implementation of Cache Mechanism
    The response will now be stored using Cache::remember(), a caching technology. This allows for quicker retrieval of information because the system remembers and reuses past responses, reducing the need for time-consuming computations and enhancing the performance of our application.

  • Code Refactoring
    The logic for retrieving values based on the dataSource has been redesigned. The refactoring process has now embedded this logic into the cache callback function, resulting in cleaner and clearer code.

@atmonshi
Copy link
Member

Thank you @mohaphez, nice work.
I added a configuration for the cache duration, but I think we need more configuration.

@mohaphez
Copy link
Contributor Author

Thank you for your response @atmonshi .

Do you have any thoughts on the code below? This code allows us to use caching and remove duplicate queries. It also enables us to flush the cache with a specific key. The downside might be the use of ->get(), but I'm not sure how much of a problem that could be. :)))

Do you have any suggestions?

public function getCollectionsValuesForResponse(Field $field, FieldResponse $resp): string
{
    $response = $resp->response;
    
    if (blank($response)) {
        return '';
    }

    if (Bolt::isJson($response)) {
        $response = json_decode($response);
    }

    // Ensure response is an array
    $response = Arr::wrap($response);

    $dataSource = (int)$field->options['dataSource'];
    $cacheKey = 'dataSource_' . $dataSource . '_response';

    // Handle default model: `Collection`
    if ($dataSource !== 0) {
        $collection = Cache::remember(
            $cacheKey,
            config('zeus-bolt.cache.collection_values'),
            fn() => BoltPlugin::getModel('Collection')::find($dataSource)
        );

        return $collection?->values
            ->whereIn('itemKey', $response)
            ->pluck('itemValue')
            ->join(', ') ?? '';
    }

    // Handle custom model class as dataSource
    if (class_exists($field->options['dataSource'])) {
        $dataSourceClass = app($field->options['dataSource']);
        $collection = Cache::remember(
            $cacheKey,
            config('zeus-bolt.cache.collection_values'),
            fn() => $dataSourceClass->getQuery()->get()
        );

        return $collection->whereIn($dataSourceClass->getKeysUsing(), $response)
            ->pluck($dataSourceClass->getValuesUsing())
            ->join(', ');
    }


    return is_array($response) ? implode(', ', $response) : (string)$response;
}

@mohaphez
Copy link
Contributor Author

I’ve tested the latest changes, and everything works perfectly. If you’re ready to merge, please feel free to proceed.

Thanks again for all your hard work!

@atmonshi
Copy link
Member

The downside might be the use of ->get(),

I have some collections with over a 100 record :) so this will be very bad.

will marge later today.

thank you.

@atmonshi atmonshi merged commit e1a727a into lara-zeus:3.x Sep 13, 2024
4 checks passed
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