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

Caching feature (or documentation) #13

Open
jahsome opened this issue Feb 13, 2019 · 6 comments
Open

Caching feature (or documentation) #13

jahsome opened this issue Feb 13, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@jahsome
Copy link

jahsome commented Feb 13, 2019

Hi there!

This package looks terrific -- kudos for putting it out.

Have you thought about supporting caching? Something where we could set a default TTL in a config would be fantastic. On the Nova end it would have to bust that cache up update as well.

If it's possible to implement this manually, e.g. at the controller level, could you please let me know how? I planned to cook it up myself and submit a PR with documentation, but at first glance through the source and documentation, I couldn't come up with a way to do it.

@einardivision
Copy link

This package saves the data in disk so that by itself is kind of cache, since it only has to find it in a file rather then do a database query.

One way i see of cacheing the results giving by this package is to edit it a little bit.
Go to src/Pages/Manager.php and define a new function:
public function getCurrent() { return $this->current; }

Then all you need to do to get the attributes from the specific site is to:
for example if we have this route "/trainers" and we need to pass in the request and a instance of the manager class you changed.

Then you can do this in your routes file (web.php)

Route::get('/', function (Request $request, Manager $page) {
   // My template name is home
    $page->load('home');
    // These are the attributes for this page
    $values = $page->getCurrent()->attributes();
   // Then you can cache it for 60 minutes(using Cache facade for simplicity)
    Cache::put('homepage', $values, 60)
   // Some logic to check the cache, etc and then return it
    return view('index')->with('values', $values)
});

This is a shit mix kind of, dont use this in production

There is a better way of doing this and that would be to initialize the Manager with the variables you stored in cache. Since when you do $page->load('home') you really dont need to pass anything into the view, you can just access it with {{ Page::get('key') }}

If you want help to figure this out, let me know.

this is just knowledge i gathered from using this package, i didnt make it 👍

@wimurk
Copy link
Contributor

wimurk commented Feb 13, 2019

@jake-harris caching the view output is a bad idea. Caching your database results is way more easier in your controller.

$users = cache()->remember('users', 120, function(){
   return User::all();
});

return view()->with(compact('users'));

@jahsome
Copy link
Author

jahsome commented Feb 13, 2019

@wimurk I'm sorry if I'm dense, but I don't see how that's relevant in the scope of this discussion.

There's nothing to cache from the database because it's not pulling it from the database, rather it's accessing the contents of a file. Ideally I'd like to cache the contents of that file, so it's not a disk IO every request on high-volume pages, e.g. the Home page.

@jahsome jahsome closed this as completed Feb 13, 2019
@jahsome
Copy link
Author

jahsome commented Feb 13, 2019

And to clarify, I'm not asking about how to cache, rather, I don't see a way to "grab" the values with the API of Page/Manager, although @einardivision seems to be on the right track with his example. I'll see if I can play with that.

@jahsome jahsome reopened this Feb 13, 2019
@toonvandenbos
Copy link
Member

toonvandenbos commented Feb 13, 2019

Hi there,

Caching would indeed be useful when the cache driver is set to anything else than file (or even database), in which case there would be hardly no difference.

I see two options:

  • Only store the objects in the cache when Laravel's configuration has a better cache driver enabled;
  • Let the developer choose if caching should be enabled by adding an option to the package's configuration file.

I would be glad to discuss this further before accepting PRs or implementing it myself.

Cheers!

@toonvandenbos toonvandenbos added the enhancement New feature or request label Feb 13, 2019
@raphcollective
Copy link
Contributor

This might be more relevant with the database source now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants