The amazon_item table is really a kind of cache. We should consider responding to cache_clear_all() by setting the timestamp on amazon_item to 0, which will then cause it to be rebuilt by cron, etc.

CommentFileSizeAuthor
#4 expire_amazon_item_items_on_cache_clear-1127462-4.patch2.89 KBwillvincent

Comments

willvincent’s picture

willvincent commented

As part of this change, ensure changing associate ID sets timestamps in DB to 0
(See #1127234: Amazon Associate ID does not change immediately in link if the setting is changed)

willvincent’s picture

willvincent commented

rfay,

Do you have any ideas how one would respond to a call to cache_clear_all()? It appears that the only way to do so is to actually make use of the cache system.

willvincent’s picture

willvincent commented

Nevermind.. I'm able to implement the appropriate code in hook_flush_caches().

willvincent’s picture

willvincent commented
Status: Active » Fixed
StatusFileSize
new expire_amazon_item_items_on_cache_clear-1127462-4.patch2.89 KB

Here's the patch.

And the change is commit.

I think the implementation is pretty slick.

1. If the associate ID is changed, all items in the amazon_item table are expired (invalidated), this is important since these entries include product URLs with the associate ID hardcoded into them.

2. On cache clear (flush all caches, drush cc all, etc) entries in the amazon_item table are expired (invalidated). But _not_ on cron run, which also fires hook_flush_caches() and calls cache_clear_all(). Force expiring items on every cron run would be silly.

3. In both instances above, a new function amazon_expire_items() is called. When called with no parameters all items in the amazon_item table are expired. However it can also be passed an array of specific ASINs to expire, and/or an array of productgroup product types to expire (ie: book, movie, etc).

While the latter functionality mentioned in point #3 isn't implemented anywhere currently, it does give contrib modules the ability to expire specific items without directly operating on the database, always good... and future functionality may need/want to make use of this too.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Excellent - hook_flush_caches() is what I was going to say too.

The one thing to watch out for is make sure views work after the flush. Of course since you're just setting the timestamp to 0, that will work ok for views. But when they get deleted, as I remember, views doesn't know how to recreate them, whereas some other uses do.

Looks to me on a casual review like it should go fine. debug_backtrace() looks like a sad way to have to figure that out :-(

willvincent’s picture

willvincent commented

it is a little sad, but I don't know of any other way.

If they're deleted, why would they be visible to views at all?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

I guess the point is that nodes and fields can have references to ASINs which will be recreated on save, etc. But views are dependent on the amazon_item table.

It's all moot though, because expiring the rows as you've done is the right approach.

willvincent’s picture

willvincent commented

Gotcha. I guess I just don't understand why views would ever get an ASIN that has been deleted.

But, as was mentioned briefly in another issue, perhaps the best solution is to change the views implementation so that it uses the amazon api. ;)

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

Yes, that would be absolutely killer.

Status: Fixed » Closed (fixed)

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