I think that the views 3.x version should use ctools.
So it should also use ctools_object_cache which is basically just a c&p of views_object_cache :)

Here is the initial patch.

Questions for myself:
- should the schema definition should be removed?
- should the update function use batch-api?
- should views or views_ui module have the dependency, because currently views_object_cache is only used for the ui.

CommentFileSizeAuthor
#12 563020_views_object_cache.patch8.02 KBdawehner
#4 563020_views_object_cache.patch9.26 KBIsland Usurper
#2 views-object-cache.patch9.38 KBdawehner
#1 views-object-cache.patch9.13 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new views-object-cache.patch9.13 KB

I thought i did the attachment :)

Needs also review.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new views-object-cache.patch9.38 KB

:16 < Heine> killes, dereine you cannot use schema API dependent functions in an update hook
13:16 < Heine> as DamZ also said
13:16 < Heine> It will break your update path when you change your schema

So i use db_query instead of drupal_write_record

stborchert’s picture

stborchert
he/him
Primary language German
commented

Without testing its functionality (and hoping I did understand what it does) I'd say the patch really looks good.
Great work!

Island Usurper’s picture

Island Usurper commented
StatusFileSize
new 563020_views_object_cache.patch9.26 KB

I tested the patch a bit, and nothing broke. :) A couple of dpm() calls showed the $view being loaded from cache or views_get_view() at the appropriate times.

I think it makes sense for the views_object_cache table to be removed from the schema. I also think making views.module dependent on ctools is the right move since we require its table in the update function. It doesn't make sense to put that update function in views_ui. I hope I've added the schema change correctly to the patch.

I'm not sure about using batch API for the update. I may be naïve in thinking that there are very few sites with more than a couple hundred views to update. Though I can also see how each row in views_object_cache can contain a large amount of data.

dawehner’s picture

dawehner
Primary language German
commented

Additional viewsobjectcache does only store the unsaved views in the ui.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented

If this introduces a dependency on CTools (which is a good thing), this should also be reflected in views.info with dependencies[]. Or did I miss something here?

dawehner’s picture

dawehner
Primary language German
commented 
+++ views.info	2009-09-05 11:40:07 +0000
@@ -2,4 +2,5 @@
 package = Views
+dependencies[] = ctools

Here it is :)

dawehner’s picture

dawehner
Primary language German
commented

The patch still applies clean. This path is quite simple, so i would like to have some people have an eye on it.

merlinofchaos’s picture

merlinofchaos commented

The only reason I haven't applied this yet is that I'd been holding off on introducing the CTools dependency until the last possible minute, since once it's in that's kind of a no-going-back thing.

That said I think we're actually pretty much at that point, so I'll take a look at that when I've got the pluggable pagers patch ready to submit.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work

This patch does not apply anymore

YK85’s picture

YK85 commented

subscribing

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 563020_views_object_cache.patch8.02 KB

Just a rerole of the patch.

dawehner’s picture

dawehner
Primary language German
commented
Version: 6.x-3.x-dev » 7.x-3.x-dev

This probably will not happen in views3 on drupal6.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Fixed

Commited in d7.

Status: Fixed » Closed (fixed)
Issue tags: -views 3.x roadmap

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +views 3.x roadmap

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.