Skip to content

Commit 150f09a

Browse files
Alfagun74Alfagun74
committed
optimizations
1 parent a52ed4a commit 150f09a

4 files changed

Lines changed: 145 additions & 115 deletions

File tree

‎src/modules/database/migrations/postgres/1771165504000-game-versions.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export class GameVersions1771165504000 implements MigrationInterface {
4242
await queryRunner.query(`
4343
INSERT INTO game_version (
4444
game_id,
45+
deleted_at,
4546
file_path,
4647
version,
4748
size,
@@ -52,6 +53,7 @@ export class GameVersions1771165504000 implements MigrationInterface {
5253
)
5354
SELECT
5455
g.id,
56+
g.deleted_at,
5557
g.file_path,
5658
g.version,
5759
g.size,

‎src/modules/database/migrations/sqlite/1771165504000-game-versions.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export class GameVersions1771165504000 implements MigrationInterface {
3535
await queryRunner.query(`
3636
INSERT OR IGNORE INTO game_version (
3737
game_id,
38+
deleted_at,
3839
file_path,
3940
version,
4041
size,
@@ -45,6 +46,7 @@ export class GameVersions1771165504000 implements MigrationInterface {
4546
)
4647
SELECT
4748
g.id,
49+
g.deleted_at,
4850
g.file_path,
4951
g.version,
5052
g.size,

‎src/modules/games/files.service.spec.ts‎

Lines changed: 59 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ jest.mock("fs-extra", () => ({
5252
describe("FilesService", () => {
5353
let service: FilesService;
5454
let configuration: {
55+
TESTING: {
56+
MOCK_FILES: boolean;
57+
};
5558
GAMES: {
5659
SEARCH_EXCLUDE_FILE_REGEX?: RegExp;
5760
SEARCH_EXCLUDE_DIR_REGEX?: RegExp;
@@ -65,6 +68,8 @@ describe("FilesService", () => {
6568
findOne: jest.Mock;
6669
save: jest.Mock;
6770
recover: jest.Mock;
71+
softDelete: jest.Mock;
72+
createQueryBuilder: jest.Mock;
6873
};
6974
let fsExtra: {
7075
access: jest.Mock;
@@ -105,6 +110,8 @@ describe("FilesService", () => {
105110
findOne: jest.fn(),
106111
save: jest.fn(),
107112
recover: jest.fn(),
113+
softDelete: jest.fn(),
114+
createQueryBuilder: jest.fn(),
108115
};
109116

110117
service = new FilesService(
@@ -415,49 +422,59 @@ describe("FilesService", () => {
415422
});
416423
});
417424

418-
describe("upsertReleaseRecord", () => {
419-
it("should recover and update a soft-deleted existing version", async () => {
420-
const existing = {
421-
id: 12,
422-
deleted_at: new Date("2026-01-01T00:00:00.000Z"),
423-
} as any;
425+
describe("checkIntegrity", () => {
426+
it("should delete migrated games that only have soft-deleted version history", async () => {
427+
configuration.TESTING.MOCK_FILES = false;
424428

425-
gameVersionRepository.findOne.mockResolvedValueOnce(existing);
429+
gamesService.find.mockResolvedValueOnce([
430+
{
431+
id: 77,
432+
file_path: "/tmp/test-files/Shared Existing File.zip",
433+
},
434+
] as any);
426435

427-
await (service as any).upsertReleaseRecord(5, {
428-
file_path: "/tmp/test-files/Game (v1).zip",
429-
version: "v1",
430-
size: 1000n,
431-
release_date: new Date("2024-01-01T00:00:00.000Z"),
432-
early_access: false,
433-
type: "WINDOWS_SETUP",
434-
});
436+
gameVersionRepository.find
437+
.mockResolvedValueOnce([
438+
{
439+
id: 900,
440+
game: { id: 152, deleted_at: new Date("2023-06-07T21:00:06.370Z") },
441+
file_path: "/files/Honey, I Joined a Cult (2021).7z",
442+
deleted_at: undefined,
443+
},
444+
] as any)
445+
.mockResolvedValueOnce([
446+
{
447+
id: 701,
448+
game: { id: 77 },
449+
file_path: "/tmp/test-files/Shared Existing File.zip",
450+
deleted_at: new Date("2026-01-01T00:00:00.000Z"),
451+
},
452+
] as any);
453+
454+
jest.spyOn(service as any, "readAllFiles").mockResolvedValueOnce([
455+
{
456+
path: "/tmp/test-files/Shared Existing File.zip",
457+
size: 123,
458+
},
459+
]);
435460

436-
expect(gameVersionRepository.findOne).toHaveBeenCalledWith(
437-
expect.objectContaining({ withDeleted: true }),
438-
);
439-
expect(gameVersionRepository.recover).toHaveBeenCalledWith(existing);
440-
expect(gameVersionRepository.save).toHaveBeenCalledWith(
441-
expect.objectContaining({
442-
id: 12,
443-
file_path: "/tmp/test-files/Game (v1).zip",
444-
version: "v1",
445-
}),
446-
);
461+
await (service as any).checkIntegrity();
462+
463+
expect(gameVersionRepository.softDelete).toHaveBeenCalledWith([900]);
464+
expect(gamesService.delete).toHaveBeenCalledWith(77);
447465
});
466+
});
448467

449-
it("should retry as update when insert collides with unique constraint", async () => {
450-
const existing = { id: 44, deleted_at: null } as any;
468+
describe("upsertReleaseRecord", () => {
469+
it("should upsert game versions atomically via query builder", async () => {
470+
const execute = jest.fn().mockResolvedValue(undefined);
471+
const orUpdate = jest.fn().mockReturnValue({ execute });
472+
const values = jest.fn().mockReturnValue({ orUpdate });
473+
const into = jest.fn().mockReturnValue({ values });
474+
const insert = jest.fn().mockReturnValue({ into });
475+
const qb = { insert };
451476

452-
gameVersionRepository.findOne
453-
.mockResolvedValueOnce(undefined)
454-
.mockResolvedValueOnce(existing);
455-
gameVersionRepository.save
456-
.mockRejectedValueOnce({
457-
code: "23505",
458-
message: "duplicate key value violates unique constraint",
459-
})
460-
.mockResolvedValueOnce(existing);
477+
gameVersionRepository.createQueryBuilder.mockReturnValue(qb as any);
461478

462479
await (service as any).upsertReleaseRecord(9, {
463480
file_path: "/tmp/test-files/Game (v2).zip",
@@ -468,16 +485,12 @@ describe("FilesService", () => {
468485
type: "WINDOWS_PORTABLE",
469486
});
470487

471-
expect(gameVersionRepository.findOne).toHaveBeenCalledTimes(2);
472-
expect(gameVersionRepository.save).toHaveBeenCalledTimes(2);
473-
expect(gameVersionRepository.save).toHaveBeenLastCalledWith(
474-
expect.objectContaining({
475-
id: 44,
476-
file_path: "/tmp/test-files/Game (v2).zip",
477-
version: "v2",
478-
early_access: true,
479-
}),
488+
expect(gameVersionRepository.createQueryBuilder).toHaveBeenCalled();
489+
expect(orUpdate).toHaveBeenCalledWith(
490+
expect.arrayContaining(["deleted_at", "updated_at"]),
491+
["game_id", "file_path"],
480492
);
493+
expect(execute).toHaveBeenCalled();
481494
});
482495
});
483496

‎src/modules/games/files.service.ts‎

Lines changed: 82 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { mergeMap } from "rxjs/operators";
2727
import filenameSanitizer from "sanitize-filename";
2828
import { Readable } from "stream";
2929
import { Throttle } from "stream-throttle";
30-
import { Repository } from "typeorm";
30+
import { IsNull, Not, Repository } from "typeorm";
3131
import unidecode from "unidecode";
3232

3333
import { Cron, SchedulerRegistry } from "@nestjs/schedule";
@@ -408,70 +408,38 @@ export class FilesService implements OnApplicationBootstrap {
408408
gameId: number,
409409
indexedGame: GamevaultGame,
410410
): Promise<void> {
411-
const query = {
412-
where: {
413-
game: { id: gameId },
411+
const now = new Date();
412+
413+
await this.gameVersionRepository
414+
.createQueryBuilder()
415+
.insert()
416+
.into(GameVersionEntity)
417+
.values({
418+
game: { id: gameId } as GamevaultGame,
414419
file_path: indexedGame.file_path,
415-
},
416-
relationLoadStrategy: "query" as const,
417-
relations: ["game"],
418-
withDeleted: true,
419-
};
420-
421-
let existingVersion = await this.gameVersionRepository.findOne(query);
422-
423-
if (!existingVersion) {
424-
const newVersion = new GameVersionEntity();
425-
newVersion.game = { id: gameId } as GamevaultGame;
426-
newVersion.file_path = indexedGame.file_path;
427-
newVersion.version = indexedGame.version;
428-
newVersion.size = indexedGame.size;
429-
newVersion.release_date = indexedGame.release_date;
430-
newVersion.early_access = indexedGame.early_access;
431-
newVersion.type = indexedGame.type;
432-
newVersion.indexed_at = new Date();
433-
434-
try {
435-
await this.gameVersionRepository.save(newVersion);
436-
return;
437-
} catch (error) {
438-
if (!this.isUniqueConstraintViolation(error)) {
439-
throw error;
440-
}
441-
442-
existingVersion = await this.gameVersionRepository.findOne(query);
443-
}
444-
}
445-
446-
if (!existingVersion) {
447-
throw new BadRequestException(
448-
"Failed to upsert game version due to a concurrent write conflict.",
449-
);
450-
}
451-
452-
if (existingVersion.deleted_at) {
453-
await this.gameVersionRepository.recover(existingVersion);
454-
}
455-
456-
existingVersion.game = { id: gameId } as GamevaultGame;
457-
existingVersion.file_path = indexedGame.file_path;
458-
existingVersion.version = indexedGame.version;
459-
existingVersion.size = indexedGame.size;
460-
existingVersion.release_date = indexedGame.release_date;
461-
existingVersion.early_access = indexedGame.early_access;
462-
existingVersion.type = indexedGame.type;
463-
existingVersion.indexed_at = new Date();
464-
await this.gameVersionRepository.save(existingVersion);
465-
}
466-
467-
private isUniqueConstraintViolation(error: unknown): boolean {
468-
const code = (error as { code?: string })?.code;
469-
if (code === "23505" || code === "SQLITE_CONSTRAINT") {
470-
return true;
471-
}
472-
473-
const message = (error as { message?: string })?.message || "";
474-
return /duplicate key value violates unique constraint/i.test(message);
420+
version: indexedGame.version,
421+
size: indexedGame.size,
422+
release_date: indexedGame.release_date,
423+
early_access: indexedGame.early_access,
424+
type: indexedGame.type,
425+
indexed_at: now,
426+
deleted_at: null,
427+
updated_at: now,
428+
})
429+
.orUpdate(
430+
[
431+
"version",
432+
"size",
433+
"release_date",
434+
"early_access",
435+
"type",
436+
"indexed_at",
437+
"deleted_at",
438+
"updated_at",
439+
],
440+
["game_id", "file_path"],
441+
)
442+
.execute();
475443
}
476444

477445
/** Returns all versions from normalized storage, falling back to legacy columns. */
@@ -894,6 +862,8 @@ export class FilesService implements OnApplicationBootstrap {
894862
count: gamesInDatabase.length,
895863
});
896864

865+
await this.cleanupDanglingVersionsForDeletedGames();
866+
897867
const fsPaths = new Set(gamesInFileSystem.map((f) => f.path));
898868
const checkedGames: GamevaultGame[] = [];
899869
for (const gameInDatabase of gamesInDatabase) {
@@ -902,11 +872,16 @@ export class FilesService implements OnApplicationBootstrap {
902872
where: { game: { id: gameInDatabase.id } },
903873
relationLoadStrategy: "query",
904874
relations: ["game"],
905-
withDeleted: false,
875+
withDeleted: true,
906876
});
877+
878+
const activePersistedVersions = persistedVersions.filter(
879+
(version) => !version.deleted_at,
880+
);
881+
907882
const availablePersistedVersions =
908-
persistedVersions.length > 0
909-
? persistedVersions.map((version) =>
883+
activePersistedVersions.length > 0
884+
? activePersistedVersions.map((version) =>
910885
Object.assign(new GameVersionEntity(), {
911886
id: version.id,
912887
game: version.game,
@@ -920,13 +895,22 @@ export class FilesService implements OnApplicationBootstrap {
920895
version.indexed_at || version.updated_at || new Date(),
921896
}),
922897
)
923-
: this.normalizeVersions(gameInDatabase);
898+
: persistedVersions.length > 0
899+
? []
900+
: this.normalizeVersions(gameInDatabase);
924901
const existingVersions = availablePersistedVersions.filter((version) =>
925902
fsPaths.has(version.file_path),
926903
);
927904

928905
// If none of the versions are available anymore, mark game as deleted.
929906
if (existingVersions.length === 0) {
907+
const activeVersionIds = activePersistedVersions.map(
908+
(version) => version.id,
909+
);
910+
if (activeVersionIds.length > 0) {
911+
await this.gameVersionRepository.softDelete(activeVersionIds);
912+
}
913+
930914
await this.gamesService.delete(gameInDatabase.id);
931915
this.logger.log({
932916
message: `Game marked as soft-deleted.`,
@@ -943,7 +927,7 @@ export class FilesService implements OnApplicationBootstrap {
943927
existingVersions.length !== availablePersistedVersions.length;
944928

945929
if (versionsChanged) {
946-
const staleVersionIds = persistedVersions
930+
const staleVersionIds = activePersistedVersions
947931
.filter((version) => !fsPaths.has(version.file_path))
948932
.map((version) => version.id);
949933

@@ -982,6 +966,35 @@ export class FilesService implements OnApplicationBootstrap {
982966
return checkedGames;
983967
}
984968

969+
/** Soft-deletes active versions that still belong to already deleted games. */
970+
private async cleanupDanglingVersionsForDeletedGames(): Promise<void> {
971+
const danglingVersions = await this.gameVersionRepository.find({
972+
where: {
973+
deleted_at: IsNull(),
974+
game: {
975+
deleted_at: Not(IsNull()),
976+
},
977+
},
978+
relationLoadStrategy: "query",
979+
relations: ["game"],
980+
withDeleted: true,
981+
});
982+
983+
const danglingVersionIds = danglingVersions
984+
.map((version) => version.id)
985+
.filter((id): id is number => Number.isFinite(id));
986+
987+
if (danglingVersionIds.length === 0) {
988+
return;
989+
}
990+
991+
await this.gameVersionRepository.softDelete(danglingVersionIds);
992+
this.logger.log({
993+
message: "Soft-deleted dangling game versions for already deleted games.",
994+
count: danglingVersionIds.length,
995+
});
996+
}
997+
985998
/** Checks whether a given filename should be included by the indexer. */
986999
private shouldIncludeFile(filename: string): boolean {
9871000
const shouldExclude =

0 commit comments

Comments
 (0)