Skip to content

Commit

Permalink
Fix user cache acting incorrectly with evictions
Browse files Browse the repository at this point in the history
The first issue involved copied users which would lead to user updates
causing faster evictions of the cache than was expected.

The second issue involved users that weren't bound to an internal
lifetime eviction policy. These users would not get evicted.
For example, a user without mutual guilds or being part of the internal
cache in general (messages, DMs) would never end up being evicted for
some strange reason. To handle this case, store_user would get a
counterpart named create_user which would create a user without
potentially storing them in the cache. That way only users with a
bound lifetime within the library would be stored.
  • Loading branch information
Rapptz committed Jul 29, 2021
1 parent 0d3bd30 commit ecf239d
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 8 deletions.
2 changes: 1 addition & 1 deletion discord/appinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def __init__(self, state: ConnectionState, data: AppInfoPayload):
self.rpc_origins: List[str] = data['rpc_origins']
self.bot_public: bool = data['bot_public']
self.bot_require_code_grant: bool = data['bot_require_code_grant']
self.owner: User = state.store_user(data['owner'])
self.owner: User = state.create_user(data['owner'])

team: Optional[TeamPayload] = data.get('team')
self.team: Optional[Team] = Team(state, team) if team else None
Expand Down
3 changes: 3 additions & 0 deletions discord/interactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,9 @@ def _get_guild(self, guild_id):
def store_user(self, data):
return self._parent.store_user(data)

def create_user(self, data):
return self._parent.create_user(data)

@property
def http(self):
return self._parent.http
Expand Down
4 changes: 2 additions & 2 deletions discord/invite.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,12 +352,12 @@ def __init__(
self.expires_at: Optional[datetime.datetime] = parse_time(expires_at) if expires_at else None

inviter_data = data.get('inviter')
self.inviter: Optional[User] = None if inviter_data is None else self._state.store_user(inviter_data)
self.inviter: Optional[User] = None if inviter_data is None else self._state.create_user(inviter_data)

self.channel: Optional[InviteChannelType] = self._resolve_channel(data.get('channel'), channel)

target_user_data = data.get('target_user')
self.target_user: Optional[User] = None if target_user_data is None else self._state.store_user(target_user_data)
self.target_user: Optional[User] = None if target_user_data is None else self._state.create_user(target_user_data)

self.target_type: InviteTarget = try_enum(InviteTarget, data.get("target_type", 0))

Expand Down
2 changes: 1 addition & 1 deletion discord/member.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def _try_upgrade(cls: Type[M], *, data: UserWithMemberPayload, guild: Guild, sta
try:
member_data = data.pop('member')
except KeyError:
return state.store_user(data)
return state.create_user(data)
else:
member_data['user'] = data # type: ignore
return cls(data=member_data, guild=guild, state=state) # type: ignore
Expand Down
4 changes: 2 additions & 2 deletions discord/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def __init__(self, *, dispatch, handlers, hooks, http, loop, **options):
self._intents = intents

if not intents.members or cache_flags._empty:
self.store_user = self.store_user_no_intents
self.store_user = self.create_user
self.deref_user = self.deref_user_no_intents

self.parsers = parsers = {}
Expand Down Expand Up @@ -284,7 +284,7 @@ def store_user(self, data):
def deref_user(self, user_id):
self._users.pop(user_id, None)

def store_user_no_intents(self, data):
def create_user(self, data):
return User(state=self, data=data)

def deref_user_no_intents(self, user_id):
Expand Down
2 changes: 1 addition & 1 deletion discord/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def _store(self, data: TemplatePayload):
self.name = data['name']
self.description = data['description']
creator_data = data.get('creator')
self.creator = None if creator_data is None else self._state.store_user(creator_data)
self.creator = None if creator_data is None else self._state.create_user(creator_data)

self.created_at = parse_time(data.get('created_at'))
self.updated_at = parse_time(data.get('updated_at'))
Expand Down
2 changes: 1 addition & 1 deletion discord/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ def __del__(self) -> None:
@classmethod
def _copy(cls, user):
self = super()._copy(user)
self._stored = getattr(user, '_stored', False)
self._stored = False
return self

async def _get_channel(self):
Expand Down
3 changes: 3 additions & 0 deletions discord/webhook/async_.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,9 @@ def store_user(self, data):
return self._parent.store_user(data)
return BaseUser(state=self, data=data)

def create_user(self, data):
return BaseUser(state=self, data=data)

@property
def http(self):
if self._parent is not None:
Expand Down

0 comments on commit ecf239d

Please sign in to comment.