Preferences model are locked to single user because of model caching

[TheRND]

Now admin user can't set preferences for different users, because Preferences model constructor has userId parameter and it's instance cached then. So getPreferencesModel with different userId is simply ignored, which is not correct. I'm suggesting you to remove userId from constructor and move it to method parameters, or respect userId constructor parameter while caching model.

[gnello]

Hi @TheRND, thanks for pointing that out!
I think is better remove userId from constructor and move it to method parameters, as you said.

I'll fix it as soon as I can.

[gnello]

Hi @TheRND,
I finally decided to not cache the model, keeping the "userId" parameter in the constructor. In this way no breaking changes were introduced.

Thanks again for pointing this out!

[TheRND]

The better way, probably, is to cache model, but implement setter method and explicitly set userId on retrieval in Dirver::getPreferencesModel($userId)?

[gnello]

Oh that could be a good idea, you mean something like this:

 /**
     * @param $userId
     * @return PreferenceModel
     */
    public function getPreferenceModel($userId)
    {
        if (!isset($this->models[PreferenceModel::class][$userId])) {
            $this->models[PreferenceModel::class][$userId] = new PreferenceModel($this->container['client'], $userId);
        }

        return $this->models[PreferenceModel::class][$userId];
    }

so every instance of a user id is cached. I wonder, however, if it does not overload the memory where there are many retrieval with different user ids.

[TheRND]

Not exactly, I meant something like this:

/**
     * @param $userId
     * @return PreferenceModel
     */
    public function getPreferenceModel($userId)
    {
        if (!isset($this->models[PreferenceModel::class])) {
            $this->models[PreferenceModel::class] = new PreferenceModel($this->container['client']); // Remove userid from constructor and move it into setter
        }

        return $this->models[PreferenceModel::class]->setUserId($userId); // Transient setter
    }

So we've got single instance cached and kept getPreferenceModel($userId) method signature for backward compatibility

[gnello]

Great! I like it, I will implement your solution this weekend.

[gnello]

Done 😊