Skip to content

fix(server/accounts): return cached account instance in OxAccount.get - #253

Open
DoluTattoo wants to merge 1 commit into
overextended:mainfrom
DoluTattoo:fix/account-cache-lookup
Open

fix(server/accounts): return cached account instance in OxAccount.get#253
DoluTattoo wants to merge 1 commit into
overextended:mainfrom
DoluTattoo:fix/account-cache-lookup

Conversation

@DoluTattoo

Copy link
Copy Markdown
Contributor

Problem

OxAccount.get never returns its cached instance:

static async get(accountId: number) {
  if (accountId in this.members) this.members[accountId];

  const validAccount = await SelectAccount(accountId);
  ...
  return new OxAccount(accountId);
}

this.members[accountId] is an expression statement, so the looked-up value is discarded. Every call falls through to SelectAccount and constructs a new OxAccount, which makes the cache write-only — the eviction interval in server/accounts/index.ts has nothing to evict that was ever read.

Because ClassInterface.add returns early when the id is already registered, the registry keeps the very first instance while every later caller receives a throwaway one. The extra instances are not leaked, but each lookup pays for a redundant SELECT.

Fix

Return the cached instance when one exists.

Behaviour

Identical, minus the redundant query:

  • DeleteAccount only sets type = 'inactive' and never deletes the row, and SelectAccount does not filter on type. So a cached accountId always still resolves in the database — the SELECT could never turn a cache hit into the No account exists error.
  • OxAccount holds no mutable state beyond accountId, so a cached instance is equivalent to a freshly constructed one.

Impact

Minor. The wasted query is a primary-key lookup, so this is a correctness/hygiene fix rather than a performance emergency. Note that GetCharacterAccount and GetGroupAccount still run SelectDefaultAccountId beforehand, so only the second query is avoided on those paths.

The cache lookup discarded its result instead of returning it, so every call fell through to SelectAccount and constructed a new OxAccount. The registry kept the first instance while callers received throwaway ones, and each lookup paid for a redundant query.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant