Currently, the actual store used to provide the expirable KV storage is supposed to be defined by the factory.keyvalue.expirable parameter, which defaults to default: keyvalue.expirable.database.

However, this fallback is duplicated in KeyValueFactory::get(), which does this:

    if (!isset($this->stores[$collection])) {
      if (isset($this->options[$collection])) {
        $service_id = $this->options[$collection];
      }
      elseif (isset($this->options[static::DEFAULT_SETTING])) {  // 'keyvalue_expirable_default', not 'default'
        $service_id = $this->options[static::DEFAULT_SETTING];
      }
      else {
        $service_id = static::DEFAULT_SERVICE;
      }

As a result, if a specific store is not provided when trying to instantiate an expirable KV store, the first and second checks fail because the default parameter is called default in core.services.yml but used as keyvalue_expirable_default in code, so the logic falls back to the value of KeyValueExpirableFactory::DEFAULT_SERVICE.

AFAICS, we have five use cases in core for this: form_cache (see #2183275: [PP-1] Use cache for $form, kv for $form_state), update.manager, update.manager, update.processor, user.private_tempstore and user.shared_tempstore.

There are two possible fixes: the simplest one is to change the factory.keyvalue.expirable section in default.services.yml, the better one is to fix that logic to use default.

Files: 

Comments

fgm created an issue. See original summary.

fgm’s picture

In addition, it seems that the SPECIFIC_PREFIX constant defined in both factories is never used as described.

fgm’s picture

Status: Active » Needs review
FileSize
1.3 KB

Let's see what happens when removing this.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jhominal’s picture

While I agree that replacing the DEFAULT_SETTING key from 'keyvalue_expirable_default' to 'default' is the logical long-term fix, what are the backwards compatibility constraints on minor Drupal releases?

I mean, there could be existing installations that override factory.keyvalue.expirable by supplying keyvalue_expirable_default - changing the name of that key would "break" these installations (by forcing them to immediately update their services.yml file).

Is that an acceptable breakage in 8.x?