Preferences model are locked to single user because of model caching about php-mattermost-driver HOT 7 CLOSED

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.

from php-mattermost-driver.

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!

from php-mattermost-driver.

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

from php-mattermost-driver.

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.

from php-mattermost-driver.

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

from php-mattermost-driver.

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

from php-mattermost-driver.

Done 😊

from php-mattermost-driver.