Fix getMemberOrNull and getGuildMembers caching only user data

[Galarzaa90]

As reported in Discord, these calls are storing the results to the user cache, so subsequent calls would not find them from cache.

When using cacheWithCachingRestFallback, this results in REST calls being made every single time to get members, unless they got cached from a gateway event.

[lukellmann]

You want to store both UserData and MemberData, see GuildEventHandler. I think it needs some extra logic (storeAndReturn and storeOnEach won't suffice if I'm not wrong).

[Galarzaa90]

You want to store both UserData and MemberData, see GuildEventHandler. I think it needs some extra logic (storeAndReturn and storeOnEach won't suffice if I'm not wrong).

For getMemberOrNull this could work:

override suspend fun getMemberOrNull(guildId: Snowflake, userId: Snowflake): Member? {
    return storeAndReturn(supplier.getMemberOrNull(guildId, userId)) {
        cache.put(it.data)
        it.memberData
    }
}

or without storeAndReturns, so it doesn't look "hacky":

override suspend fun getMemberOrNull(guildId: Snowflake, userId: Snowflake): Member? {
    return supplier.getMemberOrNull(guildId, userId)?.also { 
        cache.put(it.data)
        cache.put(it.memberData)
    }
}

For getGuildMembers, that won't work because cache.put is a suspend function and the transform on storeOnEach is crossinline.

Should additional utility functions be added for these single uses?

Alternative for getGuildMembers:

override fun getGuildMembers(guildId: Snowflake, limit: Int?): Flow<Member> {
    return supplier.getGuildMembers(guildId, limit).onEach { 
        cache.put(it.data)
        cache.put(it.memberData)
    }
}

[lukellmann]

or without storeAndReturns, so it doesn't look "hacky":

override suspend fun getMemberOrNull(guildId: Snowflake, userId: Snowflake): Member? {
    return supplier.getMemberOrNull(guildId, userId)?.also { 
        cache.put(it.data)
        cache.put(it.memberData)
    }
}

Yes, I'd prefer something like this.

For getGuildMembers, that won't work because cache.put is a suspend function and the transform on storeOnEach is crossinline.

Should additional utility functions be added for these single uses?

Alternative for getGuildMembers:

override fun getGuildMembers(guildId: Snowflake, limit: Int?): Flow<Member> {
    return supplier.getGuildMembers(guildId, limit).onEach { 
        cache.put(it.data)
        cache.put(it.memberData)
    }
}

I think we don't need an extra function and make it complicated, keeping it simple with something like your alternative should be fine.

Note that in both cases using an expression body function and a named lambda parameter instead of it might be slightly more readable IMHO.

[Galarzaa90]

Changes applied

[lukellmann]

thanks :)