Certain functions in amazon_store.module are suitable for use as an API by other applications, but they always use the currently-configured store locale (stored in the 'amazon_locale' variable) or cache policy setting (from the amazon_store_refresh_schedule variable).

Other modules can't perform an API call for a non-configured locale because they can't specify the locale in the function call parameters. Less dramatic but still important, callers can't specify a cache timeout parameter in amazon_store_cache_item() (the function reads the amazon_store_refresh_schedule variable internally).

I propose adding a $locale, $cache_timeout (and possibly other) parameters to these functions where appropriate. If the $locale or $cache_timeout parameters are not specified, the functions will behave as they do now: they will get the locale from the amazon_locale variable, or the cache timeout from amazon_store_refresh_schedule variable. Otherwise, they will use the specified parameter(s).

This simple modification would permit code reuse by other applications (or contributed add-ins for the amazon_store module) without re-inventing the existing lookup and caching mechanisms, and without impacting existing client code.

(Note: I'm aware of Amazon's policy on retaining data, but I assert that some queries are perfectly safe to retain for longer periods, like BrowseNodeLookup operation results).

Some (but not necessarily all) functions affected by this issue:

  • amazon_store_retrieve_item
  • amazon_store_cache_item
  • amazon_store_cache_search
  • amazon_store_retrieve_cached_search

If we decide to pursue this, it might be a good excuse to bump the minor revision # to 6.x-3.x-dev and do a 6.3 release (along with other changes forthcoming)

Comments

mcurry’s picture

mcurry commented
Priority: Normal » Major

Bumping priority, as this needs to be done before other things...

rfay’s picture

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

One of the always-annoying things is that Amazon and Amazon Store come at their data management from completely different directions. Amazon turns it into Drupal tables (not very well, not very flexible) and Amazon Store treats it as cached items (flexible, but not query-able).

If you're not familiar with this great divide, take a little look at Amazon before getting too far.

Also, in D7, we could conceivably just use Amazon as a back-end for field storage, views, or both.

mcurry’s picture

mcurry commented

No, not familiar with the 'great divide' but id sounds right now that I think about it.

The other thing about the Drupal caching mechanism is that it's not always all that reliable about respecting the cache timeout specified in the cache table entries (see numerous issues related to Drupal core cache). That may be fixed now, I've not been paying all that much attention to it.

I think small design improvements can be made now (6.x-2.x and related 7.x code), without major testing impact. More significant storage changes should go into a major point release, if for no other reason than the fact that they'll require more thought... unless we've already got a new (and tested) storage management scheme in the 7.x branch (forgive me, I've not studied any of that code).

rfay’s picture

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

Except for the way fields work (slight difference) D6 and D7 are identical. There may be an issue or two resolved for one and not for the other. We don't want that :-)

Anyway, they're exactly the same conceptually.

mcurry’s picture

mcurry commented

Cool. Thanks for that info. Good to know they're staying in lock-step.

mcurry’s picture

mcurry commented
Priority: Major » Normal

Looks like this will have to touch some of the core Amazon module API functions, since locale is embedded in some of those API functions...