Skip to content

refactor(chromium): BaseChromiumCookieQueryStrategy hardcodes "chrome" schema in methods shared by all Chromium subclasses #432

@mherod

Description

@mherod

Problem

BaseChromiumCookieQueryStrategy — the base class for Chrome, Edge, Arc, Opera, and OperaGX strategies — hardcodes the string "chrome" when constructing CookieQueryBuilder instances inside processFile(), processBatchFile(), and getMetaVersion(). All Chromium subclasses inherit these methods and consequently query using Chrome's schema definition, even though the class already stores the correct browser type as this.browserType.

Location: `src/core/browsers/chromium/BaseChromiumCookieQueryStrategy.ts:139, 448`
Found in: codebase scan (SOLID analysis)
Type: correctness risk / DIP violation

Context

```typescript
// processFile() — line 448
const queryBuilder = new CookieQueryBuilder("chrome"); // ← hardcoded

const queryConfig = queryBuilder.buildSelectQuery({
name,
domain,
browser: "chrome" as const, // ← hardcoded again
});

// processBatchFile() — line 139
const queryBuilder = new CookieQueryBuilder("chrome"); // ← same hardcoding

const queryOptions = specs.map((spec) => ({
name: spec.name,
domain: spec.domain,
browser: "chrome" as const, // ← hardcoded
}));
```

The class already has the correct browser type available:

```typescript
protected browserType: ChromiumBrowser; // set via constructor
```

All currently supported Chromium browsers share the same cookie schema, so the hardcoding is harmless today. However, if any Chromium variant (e.g., a future Brave support) uses a different column name or table name, this base class would need editing even though it is supposed to be generic.

Research Findings

  • Best practice: Abstract base classes should depend on their own abstract interface (in this case this.browserType) rather than concrete literals. This is the Dependency Inversion Principle applied to string constants
  • Existing pattern: CookieQueryBuilder accepts any SqlBrowserType — it is already designed to be parameterised. this.browserType is typed as ChromiumBrowser which is a subtype of SqlBrowserType
  • Complexity: Simple — replace 4 hardcoded "chrome" string literals with this.browserType

Recommended Approach

In BaseChromiumCookieQueryStrategy, replace all hardcoded "chrome" references with this.browserType:

```typescript
// processFile() — was:
const queryBuilder = new CookieQueryBuilder("chrome");
const queryConfig = queryBuilder.buildSelectQuery({ name, domain, browser: "chrome" as const });

// processFile() — becomes:
const queryBuilder = new CookieQueryBuilder(this.browserType);
const queryConfig = queryBuilder.buildSelectQuery({ name, domain, browser: this.browserType });
```

Apply the same substitution in processBatchFile() (line 139) and verify getMetaVersion() (line 519) also uses this.browserType for the buildMetaQuery call.

Acceptance Criteria

  • All new CookieQueryBuilder("chrome") calls in BaseChromiumCookieQueryStrategy are replaced with new CookieQueryBuilder(this.browserType)
  • All browser: "chrome" as const literals in the base class are replaced with browser: this.browserType
  • No other hardcoded browser string literals remain in the base class body (constructor "Chrome" strings used for logging are acceptable)
  • Existing Chrome, Edge, and Arc strategy tests still pass
  • pnpm typecheck and pnpm lint pass

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions