From 03fd342eefd286d451d2419207aea4f52c2a6c95 Mon Sep 17 00:00:00 2001 From: DAC Date: Fri, 27 Mar 2026 10:21:42 -0700 Subject: [PATCH 1/3] dup course id fix #155 there is an issue with how the course ids are being generated that is causing duplicates to be generated consistently thus preventing the creation of new courses for the site. I detailed more of this in `api/src/utils/id_generator.ts`. for now this will keep track of created course ids and pre-existing ones so that we can relyably create new ones without having to hit the database all the time and prevent potential race conditions by having each request calculate it. --- codewit/api/src/controllers/course.ts | 135 ++++++++------- codewit/api/src/main.ts | 23 +-- codewit/api/src/utils/id_generator.ts | 233 ++++++++++++++++++++++++++ 3 files changed, 313 insertions(+), 78 deletions(-) create mode 100644 codewit/api/src/utils/id_generator.ts diff --git a/codewit/api/src/controllers/course.ts b/codewit/api/src/controllers/course.ts index 1f536ce..5899d06 100644 --- a/codewit/api/src/controllers/course.ts +++ b/codewit/api/src/controllers/course.ts @@ -20,6 +20,7 @@ import { } from '../models'; import { CourseResponse } from '../typings/response.types'; import { formatCourseResponse } from '../utils/responseFormatter'; +import { generate_id, commit_id, rollback_id } from "../utils/id_generator"; async function createCourse( title: string, @@ -30,77 +31,89 @@ async function createCourse( instructors?: number[], roster?: number[] ): Promise { - return sequelize.transaction(async (transaction) => { - // acquire SHARE ROW EXCLUSIVE lock, This lock allows concurrent reads - // and locks the table against concurrent writes to avoid race conditions - // when reading the current count of courses to create a unique course id - // refer: https://www.postgresql.org/docs/16/explicit-locking.html - await sequelize.query('LOCK TABLE "courses" IN SHARE ROW EXCLUSIVE MODE', { - transaction, - }); + // will need to store the id of the generated name so that we can either + // commit or rollback + let id = 0; + + try { + let result = sequelize.transaction(async (transaction) => { + if (auto_enroll && !enrolling) { + auto_enroll = false; + } - if (auto_enroll && !enrolling) { - auto_enroll = false; - } + let [successful, name, gen_id] = generate_id(); - const course_count = await Course.count({ transaction }); - const course = await Course.create( - { - id: uniqueNamesGenerator({ - dictionaries: [adjectives, colors, animals], - separator: '-', - // use the current count of courses as the seed to ensure uniqueness - seed: course_count + 1, - }), - title, - enrolling, - auto_enroll, - }, - { transaction } - ); + if (!successful) { + // based on how the id is currently generated, it will only fail if we + // reach the max attempts or if there are no more ids to generate + throw new Error("failed to generate course id"); + } - if (language) { - const [lang] = await Language.findOrCreate({ - where: { name: language }, - transaction, - }); + id = gen_id; - await course.setLanguage(lang, { transaction }); - } - - if (modules) { - await Promise.all( - modules.map(async (moduleId, idx) => { - await course.addModule(moduleId, { - through: { ordering: idx + 1 }, - transaction, - }); - }) + const course = await Course.create( + { + id: name, + title, + enrolling, + auto_enroll, + }, + { transaction } ); - } - if (instructors) { - await course.setInstructors(instructors, { transaction }); - } + if (language) { + const [lang] = await Language.findOrCreate({ + where: { name: language }, + transaction, + }); - if (roster) { - await course.setRoster(roster, { transaction }); - } + await course.setLanguage(lang, { transaction }); + } - await course.reload({ - // eager load the instructors - include: [ - Language, - Module, - { association: Course.associations.instructors }, - { association: Course.associations.roster }, - ], - order: [[Module, CourseModules, 'ordering', 'ASC']], - transaction, + if (modules) { + await Promise.all( + modules.map(async (moduleId, idx) => { + await course.addModule(moduleId, { + through: { ordering: idx + 1 }, + transaction, + }); + }) + ); + } + + if (instructors) { + await course.setInstructors(instructors, { transaction }); + } + + if (roster) { + await course.setRoster(roster, { transaction }); + } + + await course.reload({ + // eager load the instructors + include: [ + Language, + Module, + { association: Course.associations.instructors }, + { association: Course.associations.roster }, + ], + order: [[Module, CourseModules, 'ordering', 'ASC']], + transaction, + }); + + return formatCourseResponse(course); }); - return formatCourseResponse(course); - }); + commit_id(id); + + return result; + } catch(err) { + // not going to deal with the error other than rollback the id generated if + // one was made + rollback_id(id); + + throw err; + } } async function updateCourse( diff --git a/codewit/api/src/main.ts b/codewit/api/src/main.ts index 41f4ce0..17b55d9 100644 --- a/codewit/api/src/main.ts +++ b/codewit/api/src/main.ts @@ -14,23 +14,10 @@ import { COOKIE_KEY, HOST, PORT, REDIS_HOST, REDIS_PORT } from './secrets'; import './auth/passport'; import { checkAuth } from './middleware/auth'; import { catchError, asyncHandle } from "./middleware/catch"; -// import { RedisStore } from "connect-redis"; -// import { createClient } from "redis"; +import { init } from "./utils/id_generator"; const app = express(); -// let redisClient = createClient( -// { -// url: `redis://${REDIS_HOST}:${REDIS_PORT}`, -// } -// ) -// redisClient.connect().catch(console.error) - -// let redisStore = new RedisStore({ -// client: redisClient, -// prefix: "codewit:", -// }) - app.use( session({ secret: COOKIE_KEY, @@ -60,6 +47,8 @@ app.use('/attempts', checkAuth, attemptRouter); app.use(catchError); -app.listen(PORT, HOST, async () => { - console.log(`[ ready ] http://${HOST}:${PORT}`); -}); +init() + .then(() => app.listen(PORT, HOST, async () => { + console.log(`[ ready ] http://${HOST}:${PORT}`); + })) + .catch(console.error); diff --git a/codewit/api/src/utils/id_generator.ts b/codewit/api/src/utils/id_generator.ts new file mode 100644 index 0000000..83fe675 --- /dev/null +++ b/codewit/api/src/utils/id_generator.ts @@ -0,0 +1,233 @@ +import { QueryTypes } from "sequelize"; +import { + uniqueNamesGenerator, + adjectives, + colors, + animals, +} from 'unique-names-generator'; + +import { sequelize } from "../models"; + +// this is a stop gap implementation for creating unique ids for a course using +// this library, this will not be good in the future as we will start to spend +// more time trying to find a unique id. unless the size of the dictionaries +// change (2026/03/20) then the total amount of ids that we can create is +// 22_188_920. around 5800 unique ids we can start to see about a 53% chance of +// getting a duplicate id (birthday problem). + +// also note that this will not work when more than one api server exists. this +// will only work when one server instance is created otherwise we will need to +// rely on the database more which will include more problems to deal with + +// we could do this by parsing the database errors when attempting to create a +// new course and checking for constraint errors on the index / primary key. +// we can save on network calls and database calls by having the server keep +// track of them. again as stated above, this will only work if there is a +// single instance of the server otherwise we will not know what servers are +// generating what keys and will start getting database errors thus making this +// code pointless. + +// represents a generated id that has not been commited +interface TmpId { + id: number, + ts: number, +} + +let initialized = false; +let initialized_promise = null; + +// this will track the known and commited +let known_ids: Set[] = [new Set()]; +// this will track ids that have been created but not commited +let tmp_ids: Map = new Map(); +// index to lookup the id quickly when attempting to commit a generated id +let ids_index: Map = new Map(); +// unless the size of the dictionaries change, this is a number we can actually +// count to +let max_ids = adjectives.length * colors.length * animals.length; + +// if it takes us more than this amount to create a unique id then we are going +// to bail +let max_attempts = 1000; +// this will be a counter for the amount of requests made for a new id is +// generated. +let id_count = 0; +// the interval reference for cleaning up +let interval_id = 0; + +// private function to add a name to the known_ids dictionary +function add_name(name: string) { + // encountered an error where the set was too large ~16_000_000 so we will + // append to the last Set + if (known_ids[known_ids.length - 1].size === 10_000_000) { + known_ids.push(new Set()); + } + + return known_ids[known_ids.length - 1].add(name) +} + +// private function to check if a name exists in the known_ids +function has_name(name: string) { + for (let group of known_ids) { + if (group.has(name)) { + return true; + } + } + + return false; +} + +// private function to get the total amount of known_ids +function total_names() { + let total = 0; + + for (let group of known_ids) { + total += group.size; + } + + return total; +} + +// attempt to generate a new course id, returning if the generation was +// successful, the name generated, and the id associated with the generation +// request. the number id will be used to either commit or rollback the +// generated id. this will fail if all ids have been generated or if the amount +// of generated attempts reaches the designated max +export function generate_id(): [boolean, string, number] { + if (!initialized) { + throw new Error("generated ids has not been initialized"); + } + + if (total_names() >= max_ids) { + return [false, "", 0]; + } + + let successful = false; + let id = id_count += 1; + let name: string = ""; + + let attempt = 1; + + while (attempt <= max_attempts) { + // we are not going to provide a seed for now and just the Math.random() + // that the library will default to + name = uniqueNamesGenerator({ + dictionaries: [adjectives, colors, animals], + separator: '-', + }); + + if (!has_name(name) && !tmp_ids.has(name)) { + successful = true; + break; + } + + attempt += 1; + } + + if (successful) { + ids_index.set(id, name); + tmp_ids.set(name, { + id, + ts: Date.now(), + }); + } + + return [successful, name, id]; +} + +// commits the generated id to the list of known_ids, returns false if the id +// is unknown to the system +export function commit_id(id: number): boolean { + let name = ids_index.get(id); + + if (name == null) { + return false; + } + + add_name(name); + + tmp_ids.delete(name); + ids_index.delete(id); + + return true; +} + +// rolls back the generated id to be used elsewhere, returns false if the id +// is unknown to the system +export function rollback_id(id: number): boolean { + let name = ids_index.get(id); + + if (name == null) { + return false; + } + + tmp_ids.delete(name); + ids_index.delete(id); + + return true; +} + +// queries the database for a list of all known course ids and adds them to the +// list of known_ids +async function sync_ids() { + // the database keeps track of the uids through an index so it will be unique + // and act as the source of truth for the server + const results = await sequelize.query( + "select id from courses", + { type: QueryTypes.SELECT } + ); + + for (let record of results) { + //@ts-ignore + add_name(record.id); + } +} + +// cleans up tmp_ids that are older than 30 seconds +function clean_tmp() { + let now = Date.now(); + // 30 seconds is the max lifetime for a tmp id + let max_lifetime = 30 * 1000; + let deleted = 0; + + for (let key of Object.keys(tmp_ids)) { + if (now - tmp_ids[key].ts > max_lifetime) { + ids_index.delete(tmp_ids[key].id); + tmp_ids.delete(key); + deleted += 1; + } + } + + if (deleted > 0) { + console.warn("WARN: tmp_ids are not being commited or rolled back"); + } +} + +// initializes the id generator, should be called before handling requests +export async function init(): Promise { + if (!initialized) { + if (initialized_promise != null) { + console.log("awaiting iniailization"); + + await initialized_promise; + } else { + console.log("initializing server ids"); + + initialized_promise = sync_ids(); + + await initialized_promise; + + // run cleanup of tmp_ids every 15 seconds, no this will not be exact and + // does not need to be + interval_id = setInterval(() => clean_tmp(), 15 * 1000); + + initialized = true; + } + + return true; + } else { + console.log("already initialized"); + + return false; + } +} From e2b9b3a954910a21dd4dc44b3f7219fc64c641a9 Mon Sep 17 00:00:00 2001 From: DAC Date: Fri, 27 Mar 2026 10:49:47 -0700 Subject: [PATCH 2/3] type error fix #155 fixes a type error, not sure where the type is so that it can be specified so it is being inferred. --- codewit/api/src/utils/id_generator.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codewit/api/src/utils/id_generator.ts b/codewit/api/src/utils/id_generator.ts index 83fe675..e20d6be 100644 --- a/codewit/api/src/utils/id_generator.ts +++ b/codewit/api/src/utils/id_generator.ts @@ -52,8 +52,9 @@ let max_attempts = 1000; // this will be a counter for the amount of requests made for a new id is // generated. let id_count = 0; -// the interval reference for cleaning up -let interval_id = 0; +// the interval reference for cleaning up, not sure where the type definition +// for this is so it will be unset +let interval_id; // private function to add a name to the known_ids dictionary function add_name(name: string) { From 22b5e4ab339599d8024f7544acd00ce080eb2c59 Mon Sep 17 00:00:00 2001 From: DAC Date: Tue, 7 Apr 2026 17:23:04 -0700 Subject: [PATCH 3/3] bug fixes #155 - when creating a new course, the promise was not being `await`ed and the id was thusly not being commited - changed how the tmp id cleanup was handled to use the built-in iterator to fix an issue with not properly iterating over the temp ids - updated the log for the tmp id cleanup to output how many ids it cleaned up - removed the return from `add_name` as it is not used and would return the `Set` which is not desired. --- codewit/api/src/controllers/course.ts | 2 +- codewit/api/src/utils/id_generator.ts | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/codewit/api/src/controllers/course.ts b/codewit/api/src/controllers/course.ts index 5899d06..297dd70 100644 --- a/codewit/api/src/controllers/course.ts +++ b/codewit/api/src/controllers/course.ts @@ -36,7 +36,7 @@ async function createCourse( let id = 0; try { - let result = sequelize.transaction(async (transaction) => { + let result = await sequelize.transaction(async (transaction) => { if (auto_enroll && !enrolling) { auto_enroll = false; } diff --git a/codewit/api/src/utils/id_generator.ts b/codewit/api/src/utils/id_generator.ts index e20d6be..7978bf7 100644 --- a/codewit/api/src/utils/id_generator.ts +++ b/codewit/api/src/utils/id_generator.ts @@ -64,7 +64,7 @@ function add_name(name: string) { known_ids.push(new Set()); } - return known_ids[known_ids.length - 1].add(name) + known_ids[known_ids.length - 1].add(name); } // private function to check if a name exists in the known_ids @@ -178,10 +178,16 @@ async function sync_ids() { { type: QueryTypes.SELECT } ); + let count = 0; + for (let record of results) { //@ts-ignore add_name(record.id); + + count += 1; } + + return count; } // cleans up tmp_ids that are older than 30 seconds @@ -191,16 +197,17 @@ function clean_tmp() { let max_lifetime = 30 * 1000; let deleted = 0; - for (let key of Object.keys(tmp_ids)) { - if (now - tmp_ids[key].ts > max_lifetime) { - ids_index.delete(tmp_ids[key].id); + for (let [key, data] of tmp_ids) { + if (now - data.ts > max_lifetime) { + ids_index.delete(data.id); tmp_ids.delete(key); + deleted += 1; } } if (deleted > 0) { - console.warn("WARN: tmp_ids are not being commited or rolled back"); + console.warn("WARN: tmp_ids are not being commited or rolled back. found:", deleted); } }