Add a KeyValueExpirableFactoryInterface [#2160495]

Updated: Comment #N

Problem/Motivation

The KeyValueDatabaseExpirableFactory and KeyValueNullExpirableFactory classes currently implement the KeyValueFactoryInterface. This is not correct as these factories should be returning KeyValueStoreExpirableInterface instances and not KeyValueStoreInterface instances.

Proposed resolution

Add a KeyValueExpirableFactoryInterface, so the factories mentioned above can implement this instead.

Remaining tasks

Patch, reviews.

User interface changes

None

API changes

Addition of KeyValueExpirableFactoryInterface.

Comment	File	Size	Author
#6	interdiff-2160495-6.txt	2.74 KB	damiankloip
#6	2160495-6.patch	5.76 KB	damiankloip
#6
#2	interdiff-2160495-2.txt	1.32 KB	damiankloip
#2	2160495-2.patch	3.02 KB	damiankloip
#2
#1	2160495-1.patch	3.02 KB	damiankloip
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

damiankloip CreditAttribution: damiankloip commented 20 December 2013 at 13:45

File	Size
2160495-1.patch	3.02 KB

To start with, I think we can do this.

Comment #2

damiankloip CreditAttribution: damiankloip commented 20 December 2013 at 13:49

Status:

Active

» Needs review

File	Size
2160495-2.patch	3.02 KB

interdiff-2160495-2.txt	1.32 KB

1 file was hidden/shown/deleted

File	Size
2160495-1.patch	3.02 KB

Whoops, sorry!

Comment #3

20 December 2013 at 15:02

The last submitted patch, 2: 2160495-2.patch, failed testing.

Comment #4

damiankloip CreditAttribution: damiankloip commented 20 December 2013 at 16:00

So we could get into issues with class KeyValueExpirableFactory extends KeyValueFactory {

Comment #5

Berdir

German

Switzerland

CreditAttribution: Berdir commented 20 December 2013 at 16:12

(05:09:39 PM) berdir: damiankloip: we just need to update the type hint in FormBuilder
(05:10:04 PM) berdir: damiankloip: it has keyvalue.expirable as argument, so it needs to use that interface
(05:10:19 PM) berdir: damiankloip: probably only works otherwise because KeyValueDatabaseExpirableFactory implements both

Comment #6

damiankloip CreditAttribution: damiankloip commented 20 December 2013 at 17:49

File	Size
2160495-6.patch	5.76 KB

interdiff-2160495-6.txt	2.74 KB

2 files were hidden/shown/deleted

File	Size
2160495-2.patch	3.02 KB

interdiff-2160495-2.txt	1.32 KB

In the patch in #2 I changed the KeyValueDatabaseExpirableFactory to not extend the KeyValueFactory. So now it should only implement the KeyValueDatabaseExpirableFactoryInterface.

So you're just saying we should do this? (So the KeyValueExpirableFactory extends KeyValueFactory and implements KeyValueExpirableFactoryInterface, both just having the get() method).