Skip to content

Commit

Permalink
misc: addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sheensantoscapadngan committed Oct 7, 2024
1 parent 1687d66 commit 98cca70
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 46 deletions.
12 changes: 6 additions & 6 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@
"auditlog-migration:status": "knex --knexfile ./src/db/auditlog-knexfile.ts --client pg migrate:status",
"auditlog-migration:rollback": "knex --knexfile ./src/db/auditlog-knexfile.ts migrate:rollback",
"migration:new": "tsx ./scripts/create-migration.ts",
"migration:up": "knex --knexfile ./src/db/knexfile.ts --client pg migrate:up && npm run auditlog-migration:up",
"migration:down": "knex --knexfile ./src/db/knexfile.ts --client pg migrate:down && npm run auditlog-migration:down",
"migration:list": "knex --knexfile ./src/db/knexfile.ts --client pg migrate:list && npm run auditlog-migration:list",
"migration:latest": "knex --knexfile ./src/db/knexfile.ts --client pg migrate:latest && npm run auditlog-migration:latest",
"migration:status": "knex --knexfile ./src/db/knexfile.ts --client pg migrate:status && npm run auditlog-migration:status",
"migration:rollback": "knex --knexfile ./src/db/knexfile.ts migrate:rollback && npm run auditlog-migration:rollback",
"migration:up": "npm run auditlog-migration:up && knex --knexfile ./src/db/knexfile.ts --client pg migrate:up",
"migration:down": "npm run auditlog-migration:down && knex --knexfile ./src/db/knexfile.ts --client pg migrate:down",
"migration:list": "npm run auditlog-migration:list && knex --knexfile ./src/db/knexfile.ts --client pg migrate:list",
"migration:latest": "npm run auditlog-migration:latest && knex --knexfile ./src/db/knexfile.ts --client pg migrate:latest",
"migration:status": "npm run auditlog-migration:status && knex --knexfile ./src/db/knexfile.ts --client pg migrate:status",
"migration:rollback": "npm run auditlog-migration:rollback && knex --knexfile ./src/db/knexfile.ts migrate:rollback",
"seed:new": "tsx ./scripts/create-seed-file.ts",
"seed": "knex --knexfile ./src/db/knexfile.ts --client pg seed:run",
"db:reset": "npm run migration:rollback -- --all && npm run migration:latest"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { Knex } from "knex";

import { TableName } from "../schemas";

export async function up(knex: Knex): Promise<void> {
if (await knex.schema.hasTable(TableName.AuditLog)) {
const doesProjectIdExist = await knex.schema.hasColumn(TableName.AuditLog, "projectId");
const doesOrgIdExist = await knex.schema.hasColumn(TableName.AuditLog, "orgId");
const doesProjectNameExist = await knex.schema.hasColumn(TableName.AuditLog, "projectName");

await knex.schema.alterTable(TableName.AuditLog, (t) => {
if (doesOrgIdExist) {
t.dropForeign("orgId");
}

if (doesProjectIdExist) {
t.dropForeign("projectId");
}

// add normalized field
if (!doesProjectNameExist) {
t.string("projectName");
}
});
}
}

export async function down(knex: Knex): Promise<void> {
const doesProjectIdExist = await knex.schema.hasColumn(TableName.AuditLog, "projectId");
const doesOrgIdExist = await knex.schema.hasColumn(TableName.AuditLog, "orgId");
const doesProjectNameExist = await knex.schema.hasColumn(TableName.AuditLog, "projectName");

if (await knex.schema.hasTable(TableName.AuditLog)) {
await knex.schema.alterTable(TableName.AuditLog, (t) => {
if (doesOrgIdExist) {
t.foreign("orgId").references("id").inTable(TableName.Organization).onDelete("CASCADE");
}
if (doesProjectIdExist) {
t.foreign("projectId").references("id").inTable(TableName.Project).onDelete("CASCADE");
}

// remove normalized field
if (doesProjectNameExist) {
t.dropColumn("projectName");
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,9 @@ const isUsingDedicatedAuditLogDb = Boolean(process.env.AUDIT_LOGS_DB_CONNECTION_

export async function up(knex: Knex): Promise<void> {
if (!isUsingDedicatedAuditLogDb && (await knex.schema.hasTable(TableName.AuditLog))) {
// prepare the existing audit log table for it to become a partition
const doesProjectIdExist = await knex.schema.hasColumn(TableName.AuditLog, "projectId");
const doesOrgIdExist = await knex.schema.hasColumn(TableName.AuditLog, "orgId");
const doesProjectNameExist = await knex.schema.hasColumn(TableName.AuditLog, "projectName");

await knex.schema.alterTable(TableName.AuditLog, (t) => {
// remove existing keys
t.dropPrimary();

if (doesOrgIdExist) {
t.dropForeign("orgId");
}

if (doesProjectIdExist) {
t.dropForeign("projectId");
}

// add normalized fields present in the partition table
if (!doesProjectNameExist) {
t.string("projectName");
}
});
}

Expand Down Expand Up @@ -119,7 +101,7 @@ export async function up(knex: Knex): Promise<void> {
// create partitions 4 years ahead
const partitionMonths = 4 * 12;
const partitionPromises: Promise<void>[] = [];
for (let x = 1; x < partitionMonths; x += 1) {
for (let x = 1; x <= partitionMonths; x += 1) {
partitionPromises.push(
createAuditLogPartition(
knex,
Expand All @@ -134,14 +116,14 @@ export async function up(knex: Knex): Promise<void> {
}

export async function down(knex: Knex): Promise<void> {
const result = await knex.raw(`
const partitionSearchResult = await knex.raw(`
SELECT inhrelid::regclass::text
FROM pg_inherits
WHERE inhparent::regclass::text = '${TableName.PartitionedAuditLog}'
AND inhrelid::regclass::text = '${TableName.AuditLog}'
`);

const isAuditLogAPartition = result.rows.length > 0;
const isAuditLogAPartition = partitionSearchResult.rows.length > 0;
if (isAuditLogAPartition) {
// detach audit log from partition
await knex.schema.raw(`
Expand All @@ -151,10 +133,6 @@ export async function down(knex: Knex): Promise<void> {
`);

// revert audit log modifications
const doesProjectIdExist = await knex.schema.hasColumn(TableName.AuditLog, "projectId");
const doesOrgIdExist = await knex.schema.hasColumn(TableName.AuditLog, "orgId");
const doesProjectNameExist = await knex.schema.hasColumn(TableName.AuditLog, "projectName");

if (await knex.schema.hasTable(TableName.AuditLog)) {
await knex.schema.alterTable(TableName.AuditLog, (t) => {
// we drop this first because adding to the partition results in a new primary key
Expand All @@ -164,18 +142,6 @@ export async function down(knex: Knex): Promise<void> {
t.primary(["id"], {
constraintName: "audit_logs_pkey"
});

if (doesOrgIdExist) {
t.foreign("orgId").references("id").inTable(TableName.Organization).onDelete("CASCADE");
}
if (doesProjectIdExist) {
t.foreign("projectId").references("id").inTable(TableName.Project).onDelete("CASCADE");
}

// remove normalized fields
if (doesProjectNameExist) {
t.dropColumn("projectName");
}
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions backend/src/ee/services/audit-log/audit-log-dal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import knex from "knex";

import { TDbClient } from "@app/db";
import { TableName } from "@app/db/schemas";
import { BadRequestError, DatabaseError } from "@app/lib/errors";
import { DatabaseError, GatewayTimeoutError } from "@app/lib/errors";
import { ormify, selectAllTableCols } from "@app/lib/knex";
import { logger } from "@app/lib/logger";
import { QueueName } from "@app/queue";
Expand Down Expand Up @@ -112,7 +112,7 @@ export const auditLogDALFactory = (db: TDbClient) => {
return docs;
} catch (error) {
if (error instanceof knex.KnexTimeoutError) {
throw new BadRequestError({
throw new GatewayTimeoutError({
error,
message: "Failed to fetch audit logs due to timeout. Add more search filters."
});
Expand Down
12 changes: 12 additions & 0 deletions backend/src/lib/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ export class InternalServerError extends Error {
}
}

export class GatewayTimeoutError extends Error {
name: string;

error: unknown;

constructor({ name, error, message }: { message?: string; name?: string; error?: unknown }) {
super(message || "Timeout error");
this.name = name || "GatewayTimeoutError";
this.error = error;
}
}

export class UnauthorizedError extends Error {
name: string;

Expand Down
8 changes: 7 additions & 1 deletion backend/src/server/plugins/error-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
BadRequestError,
DatabaseError,
ForbiddenRequestError,
GatewayTimeoutError,
InternalServerError,
NotFoundError,
ScimRequestError,
Expand All @@ -25,7 +26,8 @@ enum HttpStatusCodes {
Unauthorized = 401,
Forbidden = 403,
// eslint-disable-next-line @typescript-eslint/no-shadow
InternalServerError = 500
InternalServerError = 500,
GatewayTimeout = 504
}

export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider) => {
Expand All @@ -47,6 +49,10 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider
void res
.status(HttpStatusCodes.InternalServerError)
.send({ statusCode: HttpStatusCodes.InternalServerError, message: "Something went wrong", error: error.name });
} else if (error instanceof GatewayTimeoutError) {
void res
.status(HttpStatusCodes.GatewayTimeout)
.send({ statusCode: HttpStatusCodes.GatewayTimeout, message: error.message, error: error.name });
} else if (error instanceof ZodError) {
void res
.status(HttpStatusCodes.Unauthorized)
Expand Down

0 comments on commit 98cca70

Please sign in to comment.