From 23fb11d50f26c2af8dea92d9c4c5941c5a723461 Mon Sep 17 00:00:00 2001 From: Mistium <92952823+Mistium@users.noreply.github.com> Date: Sun, 1 Nov 2026 08:50:45 +0000 Subject: [PATCH 1/7] Release community, classroom, admin, game and payment locks on every exit path Every sync.lock in these domains is now followed by a deferred unlock, so an error can't leave the lock held until restart. Where work carried on after the unlock, the locked part moved into its own function, so each lock still covers the same steps and nested locks keep their order. The bans file helpers take the lock in their callers instead of handing it from lockBans to saveAndUnlockBans. --- src/admin/admin-users.osl | 17 ++-- src/admin/admin.osl | 66 ++++++------ src/admin/errors.osl | 36 ++++--- src/admin/project-moderation.osl | 40 +++++--- src/auth/auth.osl | 2 +- src/classroom/classroom-billing.osl | 16 ++- src/classroom/classroom-release.osl | 31 ++++-- src/classroom/classrooms.osl | 134 ++++++++++++++++--------- src/community/comments.osl | 88 +++++++++------- src/community/development.osl | 2 +- src/community/like-milestones.osl | 2 +- src/community/news.osl | 2 +- src/community/notifications.osl | 19 +++- src/community/playtime-diagnostics.osl | 30 +++--- src/community/remote-milestones.osl | 2 +- src/community/reports.osl | 74 ++++++++------ src/community/roadmap.osl | 2 +- src/community/spaces.osl | 2 +- src/games/game-data.osl | 37 ++++--- src/games/game-inventory.osl | 4 +- src/games/game-marketplace.osl | 9 +- src/rotur/rotur-payments.osl | 36 ++++--- src/rotur/rotur-webhooks.osl | 7 +- 23 files changed, 396 insertions(+), 262 deletions(-) diff --git a/src/admin/admin-users.osl b/src/admin/admin-users.osl index 68db3da..5b3c0f1 100644 --- a/src/admin/admin-users.osl +++ b/src/admin/admin-users.osl @@ -235,7 +235,17 @@ def handleSetAdminNote(*serve.Context c) ( c.badRequest("note is too long") return ) + boolean saved = saveAdminNoteLocked(c, key, text) + if !saved ( + c.internalError("could not save the note") + return + ) + c.json(200, {ok: true, note: adminNoteFor(key)}) +) + +def saveAdminNoteLocked(*serve.Context c, string key, string text) boolean ( sync.lock("admin-notes") + defer sync.unlock("admin-notes") object data = loadAdminNotes() object notes = data.notes.assertElse(object, {}) if text == "" ( @@ -245,12 +255,7 @@ def handleSetAdminNote(*serve.Context c) ( ) data.notes = notes boolean saved = writeFile(adminNotesFile(), data) - sync.unlock("admin-notes") - if !saved ( - c.internalError("could not save the note") - return - ) - c.json(200, {ok: true, note: adminNoteFor(key)}) + return saved ) def handleListUsers(*serve.Context c) ( diff --git a/src/admin/admin.osl b/src/admin/admin.osl index 2856ba9..ab8ad31 100644 --- a/src/admin/admin.osl +++ b/src/admin/admin.osl @@ -72,9 +72,9 @@ def bannedOwners() object ( ) // One rebuild at a time; the rest wait for it and use its result. sync.lock("banned-owners") + defer sync.unlock("banned-owners") if bansCache.has("owners") ( object built = bansCache.get("owners").assert(object) - sync.unlock("banned-owners") return built ) object bans = cachedBans().bans.assertElse(object, {}) @@ -90,7 +90,6 @@ def bannedOwners() object ( ) object banned = bannedOwnerSet(bans, idsByKey) bansCache.set("owners", banned) - sync.unlock("banned-owners") return banned ) @@ -173,19 +172,16 @@ def liftRoturBan(string username) boolean ( return res.ok or res.status == 404 ) -// The bans file is changed between these two, after any calls to Rotur, and -// re-read so nothing written meanwhile is lost. -def lockBans() object ( - sync.lock("bans") +def bansForUpdate() object ( object data = loadBans() data.unbans = (data.unbans ?? {}).assertElse(object, {}) return data ) -def saveAndUnlockBans(object data) boolean ( - boolean saved = saveBans(data) - sync.unlock("bans") - return saved +def bansSnapshotLocked() object ( + sync.lock("bans") + defer sync.unlock("bans") + return bansForUpdate() ) def banUser(string username, string reason, string actor) boolean ( @@ -194,12 +190,14 @@ def banUser(string username, string reason, string actor) boolean ( return false ) string rotur = pushRoturBan(username, reason) - object data = lockBans() + sync.lock("bans") + defer sync.unlock("bans") + object data = bansForUpdate() object bans = data.bans object unbans = data.unbans bans[key] = {reason: reason, by: actor, created: timestamp, rotur: rotur} unbans.delete(key) - return saveAndUnlockBans(data) + return saveBans(data) ) def unbanUser(string username) boolean ( @@ -210,12 +208,14 @@ def unbanUser(string username) boolean ( // Remembered until a sync has seen Rotur without it, so the sync keeps // trying if Rotur couldn't be told, and never brings the ban back. boolean lifted = liftRoturBan(username) - object data = lockBans() + sync.lock("bans") + defer sync.unlock("bans") + object data = bansForUpdate() object bans = data.bans object unbans = data.unbans bans.delete(key) unbans[key] = {at: timestamp, lifted: lifted} - return saveAndUnlockBans(data) + return saveBans(data) ) // Keeps MistWarp's bans and the Rotur App's in step, every few minutes: bans @@ -223,8 +223,7 @@ def unbanUser(string username) boolean ( // Rotur couldn't be told about yet are sent again. def syncRoturBans() ( number started = timestamp - object snapshot = lockBans() - sync.unlock("bans") + object snapshot = bansSnapshotLocked() object snapshotBans = snapshot.bans.assertElse(object, {}) object snapshotUnbans = snapshot.unbans // Unbans first, so the list read next already reflects them. @@ -266,7 +265,9 @@ def syncRoturBans() ( ) ) - object data = lockBans() + sync.lock("bans") + defer sync.unlock("bans") + object data = bansForUpdate() object bans = data.bans object unbans = data.unbans boolean changed = false @@ -344,7 +345,6 @@ def syncRoturBans() ( if changed ( saveBans(data) ) - sync.unlock("bans") ) def isSuperAdmin(string username) boolean ( @@ -938,19 +938,7 @@ def handleReportAction(*serve.Context c) ( ) ) - sync.lock("reports-storage") - array currentReports = loadReports() - for i currentReports.len ( - object currentReport = currentReports[i].assert(object) - if currentReport.id.toStr() == id ( - currentReport.resolved = true - currentReport.resolvedBy = contextString(c, "username") - currentReport.action = action - currentReport.resolvedAt = timestamp - ) - ) - boolean reportsSaved = saveReports(currentReports) - sync.unlock("reports-storage") + boolean reportsSaved = markReportActionLocked(id, contextString(c, "username"), action) if !reportsSaved ( c.internalError("could not update report") return @@ -962,3 +950,19 @@ def handleReportAction(*serve.Context c) ( addNotification(found.reporter.toStr(), contextString(c, "username"), "report_update", {action: action, reportType: found.type}) c.json(200, {ok: true}) ) + +def markReportActionLocked(string id, string resolvedBy, string action) boolean ( + sync.lock("reports-storage") + defer sync.unlock("reports-storage") + array currentReports = loadReports() + for i currentReports.len ( + object currentReport = currentReports[i].assert(object) + if currentReport.id.toStr() == id ( + currentReport.resolved = true + currentReport.resolvedBy = resolvedBy + currentReport.action = action + currentReport.resolvedAt = timestamp + ) + ) + return saveReports(currentReports) +) diff --git a/src/admin/errors.osl b/src/admin/errors.osl index 74ed10f..9c30daf 100644 --- a/src/admin/errors.osl +++ b/src/admin/errors.osl @@ -273,13 +273,17 @@ def handleRecordSiteError(*serve.Context c) ( created: timestamp, resolved: false }) - sync.lock("site-errors") - void errorDocs.insertOne(entry) - sync.unlock("site-errors") + insertSiteErrorLocked(entry) void sweepSiteErrors() c.json(201, {ok: true}) ) +def insertSiteErrorLocked(object entry) ( + sync.lock("site-errors") + defer sync.unlock("site-errors") + void errorDocs.insertOne(entry) +) + def handleAdminGetErrors(*serve.Context c) ( string show = (c.query("show") ?? "").toLower().strip() array items = errorDocs.sort("created", "desc").limit(200).all() @@ -317,16 +321,11 @@ def handleAdminResolveSiteError(*serve.Context c) ( if body.contains("resolved") ( resolved = body.resolved == true ) - sync.lock("site-errors") - object entry = (errorDocs.findById(id) ?? {}) - if entry.len == 0 ( - sync.unlock("site-errors") - c.notFound("error not found") + object res = resolveSiteErrorLocked(c, id, resolved) + if res.found != true ( return ) - entry.resolved = resolved - boolean saved = putDocument(errorDocs, id, entry) - sync.unlock("site-errors") + boolean saved = res.saved == true if saved ( c.json(200, {ok: true}) return @@ -334,14 +333,27 @@ def handleAdminResolveSiteError(*serve.Context c) ( c.internalError("could not update error") ) +def resolveSiteErrorLocked(*serve.Context c, string id, boolean resolved) object ( + sync.lock("site-errors") + defer sync.unlock("site-errors") + object entry = (errorDocs.findById(id) ?? {}) + if entry.len == 0 ( + c.notFound("error not found") + return {found: false} + ) + entry.resolved = resolved + boolean saved = putDocument(errorDocs, id, entry) + return {found: true, saved: saved} +) + def deleteAllSiteErrors() number ( sync.lock("site-errors") + defer sync.unlock("site-errors") array entries = errorDocs.all() for i entries.len ( object entry = entries[i].assert(object) void errorDocs.deleteOne({_id: entry["_id"]}) ) - sync.unlock("site-errors") return entries.len ) diff --git a/src/admin/project-moderation.osl b/src/admin/project-moderation.osl index 8a4897f..4781f4a 100644 --- a/src/admin/project-moderation.osl +++ b/src/admin/project-moderation.osl @@ -116,6 +116,7 @@ def commentsVisibleTo(array comments,*serve.Context c) array ( def renameShadowBannedUser(string oldKey, string newKey) ( sync.lock("shadowbans") + defer sync.unlock("shadowbans") object data = loadShadowBans() object users = data.users.assertElse(object, {}) if users.contains(oldKey) ( @@ -126,11 +127,11 @@ def renameShadowBannedUser(string oldKey, string newKey) ( data.users = users saveShadowBans(data) ) - sync.unlock("shadowbans") ) def removeShadowBannedUser(string key) ( sync.lock("shadowbans") + defer sync.unlock("shadowbans") object data = loadShadowBans() object users = data.users.assertElse(object, {}) if users.contains(key) ( @@ -138,7 +139,6 @@ def removeShadowBannedUser(string key) ( data.users = users saveShadowBans(data) ) - sync.unlock("shadowbans") ) def removeShadowBannedProject(string projectId) ( @@ -146,12 +146,12 @@ def removeShadowBannedProject(string projectId) ( return ) sync.lock("shadowbans") + defer sync.unlock("shadowbans") object data = loadShadowBans() object projects = data.projects.assertElse(object, {}) projects.delete(projectId) data.projects = projects saveShadowBans(data) - sync.unlock("shadowbans") ) def moderationReason(object body) string ( @@ -178,18 +178,23 @@ def handleShadowBanUser(*serve.Context c) ( c.badRequest("cannot shadow ban an admin") return ) + boolean saved = saveShadowBanUserLocked(c, key, name, reason) + if !saved ( + c.internalError("could not save the shadow ban") + return + ) + c.json(200, {ok: true, shadowBan: shadowBanEntryFor(name)}) +) + +def saveShadowBanUserLocked(*serve.Context c, string key, string name, string reason) boolean ( sync.lock("shadowbans") + defer sync.unlock("shadowbans") object data = loadShadowBans() object users = data.users.assertElse(object, {}) users[key] = {username: key, userId: userIdForUsername(name), reason: reason, by: contextString(c, "username"), created: timestamp} data.users = users boolean saved = saveShadowBans(data) - sync.unlock("shadowbans") - if !saved ( - c.internalError("could not save the shadow ban") - return - ) - c.json(200, {ok: true, shadowBan: shadowBanEntryFor(name)}) + return saved ) def handleLiftShadowBan(*serve.Context c) ( @@ -254,18 +259,23 @@ def handleShadowBanProject(*serve.Context c) ( return ) string id = project.id.toStr() + boolean saved = saveShadowBanProjectLocked(c, project, id, reason) + if !saved ( + c.internalError("could not save the shadow ban") + return + ) + c.json(200, {ok: true, project: projectResponse(project, c)}) +) + +def saveShadowBanProjectLocked(*serve.Context c, object project, string id, string reason) boolean ( sync.lock("shadowbans") + defer sync.unlock("shadowbans") object data = loadShadowBans() object projects = data.projects.assertElse(object, {}) projects[id] = {reason: reason, by: contextString(c, "username"), created: timestamp, title: project.title, owner: project.owner} data.projects = projects boolean saved = saveShadowBans(data) - sync.unlock("shadowbans") - if !saved ( - c.internalError("could not save the shadow ban") - return - ) - c.json(200, {ok: true, project: projectResponse(project, c)}) + return saved ) def handleLiftProjectShadowBan(*serve.Context c) ( diff --git a/src/auth/auth.osl b/src/auth/auth.osl index 03d0d14..e7daf0a 100644 --- a/src/auth/auth.osl +++ b/src/auth/auth.osl @@ -238,6 +238,7 @@ object editorSessionMints = {} def editorSessionMintAllowed(string userId) boolean ( sync.lock("editor-session-mints") + defer sync.unlock("editor-session-mints") object entry = (editorSessionMints[userId] ?? {}).assertElse(object, {}) if timestamp - (entry.since ?? 0).toNum() > 3600000 ( entry = {since: timestamp, count: 0} @@ -247,7 +248,6 @@ def editorSessionMintAllowed(string userId) boolean ( entry.count = entry.count.toNum() + 1 ) editorSessionMints[userId] = entry - sync.unlock("editor-session-mints") return allowed ) diff --git a/src/classroom/classroom-billing.osl b/src/classroom/classroom-billing.osl index c3e0350..03cf462 100644 --- a/src/classroom/classroom-billing.osl +++ b/src/classroom/classroom-billing.osl @@ -52,21 +52,27 @@ def handleStartClassroomTrial(*serve.Context c) ( c.json(409, {ok: false, code: "school_managed", error: "Your school manages your Classroom plan."}) return ) + if !startClassroomTrialLocked(c, userId) ( + return + ) + c.json(200, {ok: true, plan: classroomPlanFor(userId), billing: classroomBillingInfo(userId)}) +) + +def startClassroomTrialLocked(*serve.Context c, string userId) boolean ( string lockName = "classroom-plan-" ++ userId sync.lock(lockName) + defer sync.unlock(lockName) object stored = (classroomPlanDocs.findById(userId) ?? {}) if !classroomTrialAvailable(stored, false, timestamp) ( - sync.unlock(lockName) c.json(409, {ok: false, code: "trial_unavailable", error: "The free trial is for teachers who have not had a Classroom plan before."}) - return + return false ) object next = classroomTrialPlan(stored, timestamp) next._id = userId next.updatedAt = timestamp putDocument(classroomPlanDocs, userId, next) invalidateClassroomPool(userId) - sync.unlock(lockName) - c.json(200, {ok: true, plan: classroomPlanFor(userId), billing: classroomBillingInfo(userId)}) + return true ) def handleClassroomCheckout(*serve.Context c) ( @@ -139,6 +145,7 @@ def handleClassroomBillingPortal(*serve.Context c) ( def storeStripePlan(string userId, object update, number eventCreated) ( string lockName = "classroom-plan-" ++ userId sync.lock(lockName) + defer sync.unlock(lockName) object stored = (classroomPlanDocs.findById(userId) ?? {}) if stored.len == 0 ( stored = {_id: userId} @@ -152,7 +159,6 @@ def storeStripePlan(string userId, object update, number eventCreated) ( putDocument(classroomPlanDocs, userId, merged) invalidateClassroomPool(userId) ) - sync.unlock(lockName) ) def applyStripeSubscription(object subscription, number eventCreated, boolean deleted) ( diff --git a/src/classroom/classroom-release.osl b/src/classroom/classroom-release.osl index 987e3a7..eeacda5 100644 --- a/src/classroom/classroom-release.osl +++ b/src/classroom/classroom-release.osl @@ -198,12 +198,29 @@ def handleRedeemClaim(*serve.Context c) ( string studentName = student.username.toStr() string claimantId = contextString(c, "userId") string claimantName = contextString(c, "username") + object released = releaseStudentLocked(c, student, classDoc, studentId, studentName, claimantName, claimantId) + if released.ok != true ( + return + ) + array moved = released.moved.assert(array) + array trashed = released.trashed.assert(array) + invalidateClassroomPool(classDoc.ownerId.toStr()) + storageSettleSoon(claimantId) + recordClassroomAudit(classDoc["_id"].toStr(), c, "student_released", {studentId: studentId, displayName: student.displayName, to: claimantName, projects: moved.len}) + array teachers = classTeacherIds(classDoc) + for i teachers.len ( + addNotification(usernameForId(teachers[i].toStr()), claimantName, "classroom_released", {classId: classDoc["_id"], className: classDoc.name, studentName: student.displayName}) + ) + c.json(200, {ok: true, projects: moved, trashed: trashed.len}) +) + +def releaseStudentLocked(*serve.Context c, object student, object classDoc, string studentId, string studentName, string claimantName, string claimantId) object ( string lockName = "classroom-release-" ++ studentId sync.lock(lockName) + defer sync.unlock(lockName) if loadStudent(studentId).len == 0 ( - sync.unlock(lockName) c.notFound("This link has expired or was already used. Ask your teacher for a new one.") - return + return {ok: false} ) archiveSubmittedWork(student, classDoc) array moved = [] @@ -230,13 +247,5 @@ def handleRedeemClaim(*serve.Context c) ( purgeUser(studentId, studentName) deleteDocumentsByField(classroomClaimDocs, "studentId", studentId) void studentDocs.deleteOne({_id: studentId}) - sync.unlock(lockName) - invalidateClassroomPool(classDoc.ownerId.toStr()) - storageSettleSoon(claimantId) - recordClassroomAudit(classDoc["_id"].toStr(), c, "student_released", {studentId: studentId, displayName: student.displayName, to: claimantName, projects: moved.len}) - array teachers = classTeacherIds(classDoc) - for i teachers.len ( - addNotification(usernameForId(teachers[i].toStr()), claimantName, "classroom_released", {classId: classDoc["_id"], className: classDoc.name, studentName: student.displayName}) - ) - c.json(200, {ok: true, projects: moved, trashed: trashed.len}) + return {ok: true, moved: moved, trashed: trashed} ) diff --git a/src/classroom/classrooms.osl b/src/classroom/classrooms.osl index 05a096d..51d5f39 100644 --- a/src/classroom/classrooms.osl +++ b/src/classroom/classrooms.osl @@ -306,21 +306,30 @@ def handleCreateClass(*serve.Context c) ( return ) string userId = contextString(c, "userId") + object classDoc = createClassLocked(c, userId, name, loginMode) + if classDoc.len == 0 ( + return + ) + string id = classDoc["_id"].toStr() + recordClassroomAudit(id, c, "class_created", {name: name}) + c.json(201, {ok: true, class: classResponse(classDoc, c)}) +) + +def createClassLocked(*serve.Context c, string userId, string name, string loginMode) object ( string lockName = classroomPlanLock(userId) sync.lock(lockName) + defer sync.unlock(lockName) object plan = classroomPlanFor(userId) if classesForPlanOwner(userId).len >= plan.maxClasses.toNum() ( - sync.unlock(lockName) string classWord = if plan.maxClasses.toNum() == 1 then " class" else " classes" c.json(413, {ok: false, code: "class_limit", error: "Your plan allows " ++ plan.maxClasses.toStr() ++ classWord ++ ". Archive and delete a class or upgrade your plan."}) - return + return {} ) string code = generateClassCode() string slug = generateClassSlug() if code == "" or slug == "" ( - sync.unlock(lockName) c.internalError("could not create a class code, try again") - return + return {} ) string id = "cls" ++ randomString(10) object classDoc = { @@ -336,9 +345,7 @@ def handleCreateClass(*serve.Context c) ( createdAt: timestamp } void classDocs.insertOne(classDoc) - sync.unlock(lockName) - recordClassroomAudit(id, c, "class_created", {name: name}) - c.json(201, {ok: true, class: classResponse(classDoc, c)}) + return classDoc ) def handleGetClass(*serve.Context c) ( @@ -506,15 +513,26 @@ def handleTransferClass(*serve.Context c) ( return ) string classId = classDoc["_id"].toStr() + if !transferClassLocked(c, classDoc, body, classId, nextOwnerId, previousOwnerId) ( + return + ) + invalidateClassroomPool(previousOwnerId) + invalidateClassroomPool(nextOwnerId) + recordClassroomAudit(classId, c, "class_transferred", {from: previousOwnerId, to: nextOwnerId}) + addNotification(classDoc.owner.toStr(), contextString(c, "username"), "classroom_transferred", {classId: classId, className: classDoc.name}) + c.json(200, {ok: true}) +) + +def transferClassLocked(*serve.Context c, object classDoc, object body, string classId, string nextOwnerId, string previousOwnerId) boolean ( string lockName = classroomPlanLock(nextOwnerId) sync.lock(lockName) + defer sync.unlock(lockName) object plan = classroomPlanFor(nextOwnerId) object usage = classroomUsage(nextOwnerId) number incoming = studentDocs.where("classId", "==", classId).count() if usage.classes.toNum() >= plan.maxClasses.toNum() or usage.seats.toNum() + incoming > plan.seats.toNum() ( - sync.unlock(lockName) c.json(413, {ok: false, code: "seat_limit", error: "The new owner's plan does not have room for this class and its students."}) - return + return false ) array kept = [] array coTeachers = (classDoc.coTeachers ?? []).assertElse(array, []) @@ -531,12 +549,7 @@ def handleTransferClass(*serve.Context c) ( classDoc.ownerId = nextOwnerId classDoc.owner = usernameForId(nextOwnerId) putDocument(classDocs, classId, classDoc) - sync.unlock(lockName) - invalidateClassroomPool(previousOwnerId) - invalidateClassroomPool(nextOwnerId) - recordClassroomAudit(classId, c, "class_transferred", {from: previousOwnerId, to: nextOwnerId}) - addNotification(classDoc.owner.toStr(), contextString(c, "username"), "classroom_transferred", {classId: classId, className: classDoc.name}) - c.json(200, {ok: true}) + return true ) def teacherIdForUsername(string username) string ( @@ -552,14 +565,18 @@ def teacherIdForUsername(string username) string ( ) def registerStudentIdentity(string userId, string username) ( + saveStudentIdentityLocked(userId, username) + userIdsCache.set(userId, username) + userIdsCache.set("username:" ++ normalizeUsername(username), userId) +) + +def saveStudentIdentityLocked(string userId, string username) ( sync.lock("user-ids") + defer sync.unlock("user-ids") object data = loadUserIds() object users = data.users users[userId] = username saveUserIds(data) - sync.unlock("user-ids") - userIdsCache.set(userId, username) - userIdsCache.set("username:" ++ normalizeUsername(username), userId) ) def uniqueStudentUsername(string handle, string slug) string ( @@ -610,18 +627,30 @@ def handleCreateStudents(*serve.Context c) ( return ) string ownerId = classDoc.ownerId.toStr() + object made = createStudentsLocked(c, classDoc, cleaned, ownerId) + if made.ok != true ( + return + ) + string classId = made.classId.toStr() + string loginMode = made.loginMode.toStr() + array created = made.created.assert(array) + recordClassroomAudit(classId, c, "students_created", {count: created.len}) + c.json(201, {ok: true, class: classResponse(classDoc, c), loginMode: loginMode, students: created}) +) + +def createStudentsLocked(*serve.Context c, object classDoc, array cleaned, string ownerId) object ( string lockName = classroomPlanLock(ownerId) sync.lock(lockName) + defer sync.unlock(lockName) object plan = classroomPlanFor(ownerId) number seatsUsed = studentsForPlanOwner(ownerId).len if seatsUsed + cleaned.len > plan.seats.toNum() ( - sync.unlock(lockName) number free = plan.seats.toNum() - seatsUsed if free < 0 ( free = 0 ) c.json(413, {ok: false, code: "seat_limit", error: "This plan has " ++ free.toStr() ++ " free student seats. Remove students or add seats to your plan.", seats: plan.seats, used: seatsUsed}) - return + return {ok: false} ) string classId = classDoc["_id"].toStr() string loginMode = classDoc.loginMode.toStr() @@ -648,9 +677,7 @@ def handleCreateStudents(*serve.Context c) ( void studentDocs.insertOne(student) created.append({id: studentId, displayName: displayName, username: username, credentials: credentials}) ) - sync.unlock(lockName) - recordClassroomAudit(classId, c, "students_created", {count: created.len}) - c.json(201, {ok: true, class: classResponse(classDoc, c), loginMode: loginMode, students: created}) + return {ok: true, classId: classId, loginMode: loginMode, created: created} ) def handleGetClassStudent(*serve.Context c) ( @@ -829,37 +856,44 @@ def handleClassroomLogin(*serve.Context c) ( return ) string studentId = student["_id"].toStr() + object checked = checkStudentLoginLocked(c, studentId, body, failure) + if checked.ok != true ( + return + ) + student = checked.student.assert(object) + string sessionId = createSession(student.username.toStr(), studentId, true) + setAuthCookie(c, sessionId) + c.json(200, { + ok: true, + token: sessionId, + username: student.username, + displayName: student.displayName, + isStudent: true + }) +) + +def checkStudentLoginLocked(*serve.Context c, string studentId, object body, string failure) object ( string lockName = "classroom-login-" ++ studentId sync.lock(lockName) - student = loadStudent(studentId) + defer sync.unlock(lockName) + object student = loadStudent(studentId) if studentLoginLocked(student, timestamp) ( - sync.unlock(lockName) c.json(429, {ok: false, code: "login_locked", error: "Too many tries. Ask your teacher to unlock your account, or wait 15 minutes."}) - return + return {ok: false} ) if !studentCredentialsMatch(student, body) ( object next = studentLoginAfterFailure(student, timestamp) student.failedLogins = next.failedLogins student.lockedUntil = next.lockedUntil putDocument(studentDocs, studentId, student) - sync.unlock(lockName) c.json(401, {ok: false, error: failure}) - return + return {ok: false} ) student.failedLogins = 0 student.lockedUntil = 0 student.lastLoginAt = timestamp putDocument(studentDocs, studentId, student) - sync.unlock(lockName) - string sessionId = createSession(student.username.toStr(), studentId, true) - setAuthCookie(c, sessionId) - c.json(200, { - ok: true, - token: sessionId, - username: student.username, - displayName: student.displayName, - isStudent: true - }) + return {ok: true, student: student} ) def assignmentSummary(object assignment, number studentCount) object ( @@ -1142,15 +1176,25 @@ def handleStartAssignment(*serve.Context c) ( object student = found.student.assert(object) object assignment = found.assignment.assert(object) string submissionId = assignment["_id"].toStr() ++ ":" ++ student["_id"].toStr() + object started = startSubmissionLocked(c, student, assignment, submissionId) + if started.len == 0 ( + return + ) + object submission = started.submission.assert(object) + invalidateClassroomPool(classForStudentId(student["_id"].toStr()).ownerId.toStr()) + c.json(201, {ok: true, submission: submission, projectId: started.projectId}) +) + +def startSubmissionLocked(*serve.Context c, object student, object assignment, string submissionId) object ( string lockName = "classroom-submission-" ++ submissionId sync.lock(lockName) + defer sync.unlock(lockName) object existing = (submissionDocs.findById(submissionId) ?? {}) if existing.len > 0 ( auto loaded = loadProject(existing.projectId.toStr()) if loaded.ok ( - sync.unlock(lockName) c.json(200, {ok: true, submission: existing, projectId: existing.projectId}) - return + return {} ) ) string title = assignment.title.toStr().trim(1, 100) @@ -1159,9 +1203,8 @@ def handleStartAssignment(*serve.Context c) ( if starterId != "" ( auto starter = loadProject(starterId) if !starter.ok ( - sync.unlock(lockName) c.json(409, {ok: false, code: "starter_missing", error: "The starter project for this assignment is missing. Ask your teacher."}) - return + return {} ) object starterProject = starter.project.assert(object) if projectHasWorkspace(starterProject) ( @@ -1173,9 +1216,8 @@ def handleStartAssignment(*serve.Context c) ( created = createBlankStudentProject(student, title) ) if created.ok != true ( - sync.unlock(lockName) forkFailureResponse(c, created) - return + return {} ) object project = created.project.assert(object) object submission = { @@ -1193,9 +1235,7 @@ def handleStartAssignment(*serve.Context c) ( grade: "" } putDocument(submissionDocs, submissionId, submission) - sync.unlock(lockName) - invalidateClassroomPool(classForStudentId(student["_id"].toStr()).ownerId.toStr()) - c.json(201, {ok: true, submission: submission, projectId: project.id}) + return {submission: submission, projectId: project.id} ) def handleTurnInAssignment(*serve.Context c) ( diff --git a/src/community/comments.osl b/src/community/comments.osl index e5bf62e..b2f71c2 100644 --- a/src/community/comments.osl +++ b/src/community/comments.osl @@ -19,11 +19,20 @@ def createComment(string key, string author, string authorId, string content, st return {ok: false, error: refusal} ) + object res = storeCommentLocked(key, author, authorId, trimmed, parentId, kind, donationAmount, donationPaymentId) + if res.ok == true and !res.contains("duplicate") ( + roturBadgeEvent(author, "comment_posted", 0) + indexComment(key, res.comment.assert(object)) + ) + return res +) + +def storeCommentLocked(string key, string author, string authorId, string trimmed, string parentId, string kind, number donationAmount, string donationPaymentId) object ( string lockName = "comments:" ++ key sync.lock(lockName) + defer sync.unlock(lockName) string commentsPath = commentsDir ++ key ++ ".jsonl" if fs.exists(commentsPath) and fs.size(commentsPath).unwrapOr(0) + json.stringify({author: author, content: trimmed}).toBytes().len > 8388608 ( - sync.unlock(lockName) return {ok: false, error: "this comment thread is full"} ) boolean hasJsonLines = fs.exists(commentsPath) @@ -36,7 +45,6 @@ def createComment(string key, string author, string authorId, string content, st for i comments.len ( object existing = comments[i] if existing.contains("donationPaymentId") and existing.donationPaymentId.toStr() == donationPaymentId ( - sync.unlock(lockName) return {ok: true, comment: existing, parentAuthor: "", duplicate: true} ) ) @@ -53,7 +61,6 @@ def createComment(string key, string author, string authorId, string content, st ) ) if !parentOk ( - sync.unlock(lockName) return {ok: false, error: "invalid parent comment"} ) ) @@ -78,12 +85,9 @@ def createComment(string key, string author, string authorId, string content, st comments.append(comment) saved = saveComments(key, comments) ) - sync.unlock(lockName) if !saved ( return {ok: false, error: "could not save comment"} ) - roturBadgeEvent(author, "comment_posted", 0) - indexComment(key, comment) return {ok: true, comment: comment, parentAuthor: parentAuthor} ) @@ -127,6 +131,7 @@ def commentPinnedAt(object comment) number ( def setCommentPin(string key, string cid, string requester, string moderator, boolean isAdmin, boolean pinned) object ( string lockName = "comments:" ++ key sync.lock(lockName) + defer sync.unlock(lockName) array comments = loadComments(key) object target = {} boolean found = false @@ -138,11 +143,9 @@ def setCommentPin(string key, string cid, string requester, string moderator, bo ) ) if !found ( - sync.unlock(lockName) return {ok: false, error: "comment not found", status: 404} ) if commentParentId(target) != "" ( - sync.unlock(lockName) return {ok: false, error: "only top-level comments can be pinned", status: 400} ) @@ -150,7 +153,6 @@ def setCommentPin(string key, string cid, string requester, string moderator, bo boolean allowed = isAdmin allowed = allowed or requesterKey == normalizeUsername(moderator) if !allowed ( - sync.unlock(lockName) return {ok: false, error: "not allowed", status: 403} ) @@ -171,7 +173,6 @@ def setCommentPin(string key, string cid, string requester, string moderator, bo ) ) boolean saved = saveComments(key, comments) - sync.unlock(lockName) if !saved ( return {ok: false, error: "could not save comment", status: 500} ) @@ -181,6 +182,7 @@ def setCommentPin(string key, string cid, string requester, string moderator, bo def deleteComment(string key, string cid, string requester, string moderator, boolean isAdmin) object ( string lockName = "comments:" ++ key sync.lock(lockName) + defer sync.unlock(lockName) array comments = loadComments(key) object target = {} boolean found = false @@ -192,7 +194,6 @@ def deleteComment(string key, string cid, string requester, string moderator, bo ) ) if !found ( - sync.unlock(lockName) return {ok: false, error: "comment not found", status: 404} ) @@ -201,7 +202,6 @@ def deleteComment(string key, string cid, string requester, string moderator, bo allowed = allowed or requesterKey == normalizeUsername(target.author.toStr()) allowed = allowed or requesterKey == normalizeUsername(moderator) if !allowed ( - sync.unlock(lockName) return {ok: false, error: "not allowed", status: 403} ) @@ -215,7 +215,6 @@ def deleteComment(string key, string cid, string requester, string moderator, bo ) ) boolean saved = saveComments(key, next) - sync.unlock(lockName) if !saved ( return {ok: false, error: "could not delete comment", status: 500} ) @@ -231,8 +230,17 @@ def editComment(string key, string cid, string requester, string content) object return {ok: false, error: "comment too long", status: 400} ) + object res = editCommentLocked(key, cid, requester, trimmed) + if res.ok == true ( + indexComment(key, res.comment.assert(object)) + ) + return res +) + +def editCommentLocked(string key, string cid, string requester, string trimmed) object ( string lockName = "comments:" ++ key sync.lock(lockName) + defer sync.unlock(lockName) array comments = loadComments(key) object target = {} boolean found = false @@ -244,11 +252,9 @@ def editComment(string key, string cid, string requester, string content) object ) ) if !found ( - sync.unlock(lockName) return {ok: false, error: "comment not found", status: 404} ) if normalizeUsername(requester) != normalizeUsername(target.author.toStr()) ( - sync.unlock(lockName) return {ok: false, error: "not allowed", status: 403} ) for i comments.len ( @@ -261,11 +267,9 @@ def editComment(string key, string cid, string requester, string content) object ) ) boolean saved = saveComments(key, comments) - sync.unlock(lockName) if !saved ( return {ok: false, error: "could not edit comment", status: 500} ) - indexComment(key, target) return {ok: true, comment: target} ) @@ -774,8 +778,37 @@ def reactToComment(*serve.Context c, string key, string path) ( ) string username = normalizeUsername(contextString(c, "username")) string cid = c.param("cid") + object reaction = saveCommentReactionLocked(c, key, cid, username, reactionType) + if reaction.found != true ( + return + ) + boolean saved = reaction.saved == true + object summary = reaction.summary.assert(object) + number previousLikes = reaction.previousLikes.toNum() + string author = reaction.author.toStr() + string preview = reaction.preview.toStr() + if !saved ( + c.internalError("could not save reaction") + return + ) + notifyLikeMilestones(author, "comment", key ++ ":" ++ cid, previousLikes, summary.counts.heart.toNum(), path ++ "#comment-id-" ++ cid, {commentId: cid, preview: preview}) + if key.startsWith("project-") ( + siteBroadcastProject(c.param("id"), { + type: "comment_reaction", + projectId: c.param("id"), + commentId: cid, + reactionCounts: summary.counts, + actor: contextString(c, "username"), + reaction: summary.myReaction + }) + ) + c.json(200, {ok: true, reactionCounts: summary.counts, myReaction: summary.myReaction}) +) + +def saveCommentReactionLocked(*serve.Context c, string key, string cid, string username, string reactionType) object ( string lockName = "comments:" ++ key sync.lock(lockName) + defer sync.unlock(lockName) array comments = loadComments(key) boolean found = false object summary = {counts: {heart: 0, brokenheart: 0}, myReaction: ""} @@ -799,28 +832,11 @@ def reactToComment(*serve.Context c, string key, string path) ( ) ) if !found ( - sync.unlock(lockName) c.notFound("comment not found") - return + return {found: false} ) boolean saved = saveComments(key, comments) - sync.unlock(lockName) - if !saved ( - c.internalError("could not save reaction") - return - ) - notifyLikeMilestones(author, "comment", key ++ ":" ++ cid, previousLikes, summary.counts.heart.toNum(), path ++ "#comment-id-" ++ cid, {commentId: cid, preview: preview}) - if key.startsWith("project-") ( - siteBroadcastProject(c.param("id"), { - type: "comment_reaction", - projectId: c.param("id"), - commentId: cid, - reactionCounts: summary.counts, - actor: contextString(c, "username"), - reaction: summary.myReaction - }) - ) - c.json(200, {ok: true, reactionCounts: summary.counts, myReaction: summary.myReaction}) + return {found: true, saved: saved, summary: summary, previousLikes: previousLikes, author: author, preview: preview} ) def handleReactProjectComment(*serve.Context c) ( diff --git a/src/community/development.osl b/src/community/development.osl index 0638d71..ec1d3cb 100644 --- a/src/community/development.osl +++ b/src/community/development.osl @@ -137,8 +137,8 @@ def refreshDevelopmentPullsLocked() object ( def refreshDevelopmentPulls() object ( sync.lock("development-refresh") + defer sync.unlock("development-refresh") object snapshot = refreshDevelopmentPullsLocked() - sync.unlock("development-refresh") return snapshot ) diff --git a/src/community/like-milestones.osl b/src/community/like-milestones.osl index ab0ea77..21eeff6 100644 --- a/src/community/like-milestones.osl +++ b/src/community/like-milestones.osl @@ -15,6 +15,7 @@ def notifyLikeMilestones(string target, string kind, string contentId, number be string key = kind ++ ":" ++ contentId ++ ":" ++ recipient string lockName = "like-milestone:" ++ key sync.lock(lockName) + defer sync.unlock(lockName) object previous = (likeMilestoneDocs.findById(key) ?? {}) number notifiedThrough = (previous.notifiedThrough ?? 0).toNum() array milestones = crossedLikeMilestones(before, after, notifiedThrough) @@ -37,5 +38,4 @@ def notifyLikeMilestones(string target, string kind, string contentId, number be addNotification(target, "", notifType, payload) ) ) - sync.unlock(lockName) ) diff --git a/src/community/news.osl b/src/community/news.osl index 0f6697d..01da235 100644 --- a/src/community/news.osl +++ b/src/community/news.osl @@ -462,8 +462,8 @@ def handleDeleteNews(*serve.Context c) ( def handleReactNews(*serve.Context c) ( string lockName = "news-reactions" sync.lock(lockName) + defer sync.unlock(lockName) handleReactNewsLocked(c) - sync.unlock(lockName) ) def handleReactNewsLocked(*serve.Context c) ( diff --git a/src/community/notifications.osl b/src/community/notifications.osl index c1ad591..f6f2b49 100644 --- a/src/community/notifications.osl +++ b/src/community/notifications.osl @@ -369,6 +369,7 @@ def addNotification(string target, string actor, string notifType, object extra) ) sync.lock("notifications:" ++ targetKey) + defer sync.unlock("notifications:" ++ targetKey) array items = loadNotifications(targetKey) array next = [item] number kept = 1 @@ -395,13 +396,18 @@ def addNotification(string target, string actor, string notifType, object extra) ) ) saveNotifications(targetKey, next) - sync.unlock("notifications:" ++ targetKey) ) def handleGetNotifications(*serve.Context c) ( string username = contextString(c, "username") syncFollowerMilestones(username) + array local = refreshNotificationActorsLocked(username) + c.json(200, {ok: true, notifications: local}) +) + +def refreshNotificationActorsLocked(string username) array ( sync.lock("notifications:" ++ normalizeUsername(username)) + defer sync.unlock("notifications:" ++ normalizeUsername(username)) array local = loadNotifications(username) boolean changed = false for i local.len ( @@ -421,19 +427,22 @@ def handleGetNotifications(*serve.Context c) ( if changed ( saveNotifications(username, local) ) - sync.unlock("notifications:" ++ normalizeUsername(username)) - c.json(200, {ok: true, notifications: local}) + return local ) def handleReadNotifications(*serve.Context c) ( string username = contextString(c, "username") + markNotificationsReadLocked(username) + c.json(200, {ok: true}) +) + +def markNotificationsReadLocked(string username) ( sync.lock("notifications:" ++ normalizeUsername(username)) + defer sync.unlock("notifications:" ++ normalizeUsername(username)) array items = loadNotifications(username) for i items.len ( object item = items[i] item.read = true ) saveNotifications(username, items) - sync.unlock("notifications:" ++ normalizeUsername(username)) - c.json(200, {ok: true}) ) diff --git a/src/community/playtime-diagnostics.osl b/src/community/playtime-diagnostics.osl index dabee84..34596ee 100644 --- a/src/community/playtime-diagnostics.osl +++ b/src/community/playtime-diagnostics.osl @@ -38,6 +38,7 @@ def touchRecentProjectActivity(string projectId, string username) boolean ( string playtimeId = projectId ++ ":" ++ identityKey string playtimeLock = "playtime:" ++ playtimeId sync.lock(playtimeLock) + defer sync.unlock(playtimeLock) object playtime = (playtimeDocs.findById(playtimeId) ?? {}) if playtime.len == 0 ( playtime = {_id: playtimeId, projectId: projectId, username: username, userId: userId, duration: 0, updated: timestamp} @@ -48,7 +49,6 @@ def touchRecentProjectActivity(string projectId, string username) boolean ( ) playtime.projectId = projectId boolean saved = putDocument(playtimeDocs, playtimeId, playtime) - sync.unlock(playtimeLock) return saved ) @@ -62,17 +62,7 @@ def creditPlaytimeSession(string projectId, string username, string sessionId, b if credited > 0 ( string identityKey = session.identityKey.assertElse(string, playtimeIdentity(username)) string playtimeId = projectId ++ ":" ++ identityKey - string playtimeLock = "playtime:" ++ playtimeId - sync.lock(playtimeLock) - object playtime = (playtimeDocs.findById(playtimeId) ?? {}) - if playtime.len == 0 ( - playtime = {_id: playtimeId, projectId: projectId, username: username, userId: userId, duration: 0, updated: timestamp} - ) - playtime.duration = playtime.duration.toNum() + credited - playtime.updated = timestamp - playtime.projectId = projectId - putDocument(playtimeDocs, playtimeId, playtime) - sync.unlock(playtimeLock) + creditPlaytimeLocked(playtimeId, projectId, username, userId, credited) ) if finished ( void playtimeSessionDocs.deleteOne({_id: sessionId}) @@ -84,6 +74,20 @@ def creditPlaytimeSession(string projectId, string username, string sessionId, b return credited ) +def creditPlaytimeLocked(string playtimeId, string projectId, string username, string userId, number credited) ( + string playtimeLock = "playtime:" ++ playtimeId + sync.lock(playtimeLock) + defer sync.unlock(playtimeLock) + object playtime = (playtimeDocs.findById(playtimeId) ?? {}) + if playtime.len == 0 ( + playtime = {_id: playtimeId, projectId: projectId, username: username, userId: userId, duration: 0, updated: timestamp} + ) + playtime.duration = playtime.duration.toNum() + credited + playtime.updated = timestamp + playtime.projectId = projectId + putDocument(playtimeDocs, playtimeId, playtime) +) + def recordProjectDiagnostic(string projectId, boolean authenticated, string username, string sessionId, object body) object ( string eventType = body.type.toStr().toLower().strip() if eventType != "load" and eventType != "start" and eventType != "crash" and eventType != "exit" and eventType != "playtime" and eventType != "playtime_start" and eventType != "playtime_ping" and eventType != "playtime_finish" ( @@ -128,6 +132,7 @@ def recordProjectDiagnostic(string projectId, boolean authenticated, string user object item = {_id: diagnosticId, projectId: projectId, type: eventType, duration: duration, loadMs: loadMs, error: errorText, device: device, created: timestamp} string diagnosticLock = "project-diagnostics-" ++ projectId sync.lock(diagnosticLock) + defer sync.unlock(diagnosticLock) void diagnosticDocs.insertOne(item) array storedDiagnostics = diagnosticDocs.where("projectId", "==", projectId).sort("created", "desc").limit(550).all() for i storedDiagnostics.len ( @@ -135,7 +140,6 @@ def recordProjectDiagnostic(string projectId, boolean authenticated, string user void diagnosticDocs.deleteOne({_id: storedDiagnostics[i]["_id"]}) ) ) - sync.unlock(diagnosticLock) return {ok: true} ) diff --git a/src/community/remote-milestones.osl b/src/community/remote-milestones.osl index fa3aea7..543334e 100644 --- a/src/community/remote-milestones.osl +++ b/src/community/remote-milestones.osl @@ -25,6 +25,7 @@ def milestoneDataFromUrl(string endpoint) object ( def observeRemoteMilestone(string target, string kind, string id, number count, string path, object extra) ( string key = "observed:" ++ kind ++ ":" ++ id sync.lock(key) + defer sync.unlock(key) object previous = (likeMilestoneDocs.findById(key) ?? {}) // First observation establishes a baseline; an exact milestone still counts. number before = count - 1 @@ -33,7 +34,6 @@ def observeRemoteMilestone(string target, string kind, string id, number count, ) notifyLikeMilestones(target, kind, id, before, count, path, extra) putDocument(likeMilestoneDocs, key, {count: count}) - sync.unlock(key) ) def syncFollowerMilestones(string username) ( diff --git a/src/community/reports.osl b/src/community/reports.osl index 712e4f0..fd11452 100644 --- a/src/community/reports.osl +++ b/src/community/reports.osl @@ -150,6 +150,7 @@ def reportedContent(object report) object ( def setReportRotur(string id, string state, string roturReportId) object ( sync.lock("reports-storage") + defer sync.unlock("reports-storage") array reports = loadReports() object found = {} for i reports.len ( @@ -165,7 +166,6 @@ def setReportRotur(string id, string state, string roturReportId) object ( ) ) saveReports(reports) - sync.unlock("reports-storage") return found ) @@ -297,17 +297,11 @@ def handleCreateReport(*serve.Context c) ( if roturAppReady() ( report.rotur = "pending" ) - sync.lock("reports-storage") - array reports = loadReports() - if !canAppendPendingReport(reports, report.sourceKey.toStr()) ( - sync.unlock("reports-storage") - c.json(429, {ok: false, error: "too many pending reports"}) + string appended = appendPendingReportLocked(c, report, "too many pending reports") + if appended == "full" ( return ) - reports.append(report) - boolean saved = saveReports(reports) - sync.unlock("reports-storage") - if !saved ( + if appended != "saved" ( c.internalError("could not save report") return ) @@ -347,25 +341,32 @@ def handleCreateSupportRequest(*serve.Context c) ( created: timestamp, resolved: false } - sync.lock("reports-storage") - array reports = loadReports() - if !canAppendPendingReport(reports, report.sourceKey.toStr()) ( - sync.unlock("reports-storage") - c.json(429, {ok: false, error: "too many pending support requests"}) + string appended = appendPendingReportLocked(c, report, "too many pending support requests") + if appended == "full" ( return ) - reports.append(report) - boolean saved = saveReports(reports) - sync.unlock("reports-storage") - if !saved ( + if appended != "saved" ( c.internalError("could not save support request") return ) c.json(201, {ok: true, id: report.id}) ) +def appendPendingReportLocked(*serve.Context c, object report, string fullMessage) string ( + sync.lock("reports-storage") + defer sync.unlock("reports-storage") + array reports = loadReports() + if !canAppendPendingReport(reports, report.sourceKey.toStr()) ( + c.json(429, {ok: false, error: fullMessage}) + return "full" + ) + reports.append(report) + return if saveReports(reports) then "saved" else "unsaved" +) + def updateReport(string id, object fields) boolean ( sync.lock("reports-storage") + defer sync.unlock("reports-storage") array reports = loadReports() boolean changed = false array keys = fields.getKeys() @@ -380,7 +381,6 @@ def updateReport(string id, object fields) boolean ( ) ) boolean saved = changed and saveReports(reports) - sync.unlock("reports-storage") return saved ) @@ -475,7 +475,25 @@ def handleGetReports(*serve.Context c) ( def handleResolveReport(*serve.Context c) ( object body = requestJSON(c) string id = body.id.toStr() + object res = resolveReportLocked(c, id) + if res.found != true ( + return + ) + if res.saved != true ( + c.internalError("could not update report") + return + ) + string projectTarget = res.projectTarget.toStr() + if projectTarget != "" ( + maybeClearProjectEvidence(projectTarget) + ) + closeRoturReport(res.resolved.assert(object), "resolved") + c.json(200, {ok: true}) +) + +def resolveReportLocked(*serve.Context c, string id) object ( sync.lock("reports-storage") + defer sync.unlock("reports-storage") array reports = loadReports() boolean found = false string projectTarget = "" @@ -492,21 +510,11 @@ def handleResolveReport(*serve.Context c) ( ) ) if !found ( - sync.unlock("reports-storage") c.notFound("report not found") - return + return {found: false} ) boolean saved = saveReports(reports) - sync.unlock("reports-storage") - if !saved ( - c.internalError("could not update report") - return - ) - if projectTarget != "" ( - maybeClearProjectEvidence(projectTarget) - ) - closeRoturReport(resolved, "resolved") - c.json(200, {ok: true}) + return {found: true, saved: saved, projectTarget: projectTarget, resolved: resolved} ) // Reports someone filed stay for moderation, without saying who filed them. @@ -514,6 +522,7 @@ def anonymiseReportsBy(string username, string userId) ( string key = normalizeUsername(username) string sourceKey = "user:" ++ userId sync.lock("reports-storage") + defer sync.unlock("reports-storage") array reports = loadReports() boolean changed = false for i reports.len ( @@ -528,5 +537,4 @@ def anonymiseReportsBy(string username, string userId) ( if changed ( saveReports(reports) ) - sync.unlock("reports-storage") ) diff --git a/src/community/roadmap.osl b/src/community/roadmap.osl index 0f13b0e..4a7b28b 100644 --- a/src/community/roadmap.osl +++ b/src/community/roadmap.osl @@ -97,8 +97,8 @@ def handleCreateIdea(*serve.Context c) ( def handleVoteIdea(*serve.Context c) ( string lockName = "idea-reactions:" ++ c.param("id") sync.lock(lockName) + defer sync.unlock(lockName) handleVoteIdeaLocked(c) - sync.unlock(lockName) ) def handleVoteIdeaLocked(*serve.Context c) ( diff --git a/src/community/spaces.osl b/src/community/spaces.osl index 1731725..18864dd 100644 --- a/src/community/spaces.osl +++ b/src/community/spaces.osl @@ -693,8 +693,8 @@ def handleCancelSpaceCuratorInvite(*serve.Context c) ( def handleReactSpace(*serve.Context c) ( string lockName = "space-reactions:" ++ c.param("id") sync.lock(lockName) + defer sync.unlock(lockName) handleReactSpaceLocked(c) - sync.unlock(lockName) ) def handleReactSpaceLocked(*serve.Context c) ( diff --git a/src/games/game-data.osl b/src/games/game-data.osl index 64d6cf1..b6aa37c 100644 --- a/src/games/game-data.osl +++ b/src/games/game-data.osl @@ -300,19 +300,27 @@ def handlePutMyProjectSave(*serve.Context c) ( string projectId = c.param("id") string userId = contextString(c, "userId") string context = capability.context.toStr() + object stored = storeGameSaveLocked(c, body, value, requestId, projectId, userId, context) + if stored.stored != true ( + return + ) + object current = stored.save.assert(object) + c.json(200, {ok: true, save: gameSaveResponse(current)}) +) + +def storeGameSaveLocked(*serve.Context c, object body, object value, string requestId, string projectId, string userId, string context) object ( string lockName = "game-save-storage-" ++ gameDataUserKey(userId) sync.lock(lockName) + defer sync.unlock(lockName) object current = loadGameSave(projectId, userId, context) if current.lastRequestId.toStr() == requestId ( - sync.unlock(lockName) c.json(200, {ok: true, replayed: true, save: gameSaveResponse(current)}) - return + return {stored: false} ) number revision = body.revision.toNum().toInt() if revision != current.revision.toNum().toInt() ( - sync.unlock(lockName) c.json(409, {ok: false, error: "save revision is stale", save: gameSaveResponse(current)}) - return + return {stored: false} ) current.value = value @@ -326,24 +334,20 @@ def handlePutMyProjectSave(*serve.Context c) ( number projectedBytes = (usedBytes - previousBytes) + nextBytes number storageLimit = mistwarpBenefitsForUserId(userId).gameSaveStorageBytes.toNum() if projectedBytes > storageLimit and !contextBool(c, "isAdmin") ( - sync.unlock(lockName) c.json(413, {ok: false, error: "account save storage limit reached", used: usedBytes, limit: storageLimit}) - return + return {stored: false} ) if !saveGameSaveUsage(userId, projectedBytes) ( - sync.unlock(lockName) c.internalError("failed to reserve account save storage") - return + return {stored: false} ) void fs.mkdir(gameSaveProjectDir(projectId, context), {recursive: true}) if !writeFile(path, current) ( saveGameSaveUsage(userId, usedBytes) - sync.unlock(lockName) c.internalError("failed to store project save") - return + return {stored: false} ) - sync.unlock(lockName) - c.json(200, {ok: true, save: gameSaveResponse(current)}) + return {stored: true, save: current} ) def handleListMyGameSaves(*serve.Context c) ( @@ -379,8 +383,14 @@ def handleDeleteMyGameSave(*serve.Context c) ( return ) string userId = contextString(c, "userId") + boolean deleted = deleteGameSavesLocked(projectId, userId) + c.json(200, {ok: true, deleted: deleted}) +) + +def deleteGameSavesLocked(string projectId, string userId) boolean ( string lockName = "game-save-storage-" ++ gameDataUserKey(userId) sync.lock(lockName) + defer sync.unlock(lockName) string playPath = gameSavePath(projectId, userId, "play") string testPath = gameSavePath(projectId, userId, "editor") number removedBytes = 0 @@ -398,8 +408,7 @@ def handleDeleteMyGameSave(*serve.Context c) ( number nextBytes = if usedBytes > removedBytes then usedBytes - removedBytes else 0 saveGameSaveUsage(userId, nextBytes) ) - sync.unlock(lockName) - c.json(200, {ok: true, deleted: deleted}) + return deleted ) def collectGameDataForUser(string userId) object ( diff --git a/src/games/game-inventory.osl b/src/games/game-inventory.osl index d40dc68..65c23a9 100644 --- a/src/games/game-inventory.osl +++ b/src/games/game-inventory.osl @@ -149,16 +149,15 @@ def grantInventoryItem(string userId, object project, object item, number amount string globalId = globalGameItemId(project.id.toStr(), item.id.toStr()) string lockName = "game-inventory-" ++ gameDataUserKey(userId) sync.lock(lockName) + defer sync.unlock(lockName) object inventory = loadGameInventory(userId) object items = inventory.items.assertElse(object, {}) object owned = if items.contains(globalId) then items[globalId].assertElse(object, {}) else {} array grantIds = owned.grantIds.assertElse(array, []) if owned.lastGrantId.toStr() == sourceId or grantIds.contains(sourceId) ( - sync.unlock(lockName) return {ok: true, replayed: true, item: owned} ) if !items.contains(globalId) and items.getKeys().len >= mistwarpBenefitsForUserId(userId).maxGameInventoryItems.toNum() ( - sync.unlock(lockName) return {ok: false, error: "portable item inventory limit reached"} ) number current = owned.quantity.toNum().toInt() @@ -184,7 +183,6 @@ def grantInventoryItem(string userId, object project, object item, number amount inventory.revision = inventory.revision.toNum().toInt() + 1 inventory.updatedAt = timestamp boolean saved = writeFile(gameInventoryPath(userId), inventory) - sync.unlock(lockName) if !saved ( return {ok: false, error: "failed to store portable item"} ) diff --git a/src/games/game-marketplace.osl b/src/games/game-marketplace.osl index a59d413..495902c 100644 --- a/src/games/game-marketplace.osl +++ b/src/games/game-marketplace.osl @@ -70,6 +70,7 @@ def grantGameProductItem(object project, object product, string userId, string p def saveGameProductEntitlement(string projectId, string userId, string productId, string paymentId) boolean ( string lockName = "game-entitlement-" ++ projectId ++ "-" ++ gameDataUserKey(userId) sync.lock(lockName) + defer sync.unlock(lockName) object entitlements = loadGameEntitlements(projectId, userId) object products = entitlements.products.assertElse(object, {}) products[productId] = {purchasedAt: timestamp, paymentId: paymentId} @@ -77,7 +78,6 @@ def saveGameProductEntitlement(string projectId, string userId, string productId entitlements.updatedAt = timestamp void fs.mkdir(gameEntitlementsDir ++ projectId ++ "/", {recursive: true}) boolean saved = writeFile(gameEntitlementPath(projectId, userId), entitlements) - sync.unlock(lockName) return saved ) @@ -87,6 +87,7 @@ def removeGameProductEntitlement(string projectId, string userId, string product ) string lockName = "game-entitlement-" ++ projectId ++ "-" ++ gameDataUserKey(userId) sync.lock(lockName) + defer sync.unlock(lockName) object entitlements = loadGameEntitlements(projectId, userId) object products = entitlements.products.assertElse(object, {}) products.delete(productId) @@ -94,7 +95,6 @@ def removeGameProductEntitlement(string projectId, string userId, string product entitlements.updatedAt = timestamp void fs.mkdir(gameEntitlementsDir ++ projectId ++ "/", {recursive: true}) boolean saved = writeFile(gameEntitlementPath(projectId, userId), entitlements) - sync.unlock(lockName) return saved ) @@ -191,16 +191,15 @@ def handleGameProductIntent(*serve.Context c) ( // gets here first. def completeGameProductPurchase(string key) object ( sync.lock("purchase:" ++ key) + defer sync.unlock("purchase:" ++ key) object entry = paidPurchase(key) auto loaded = loadProject((entry.projectId ?? "").toStr()) if !entry.contains("paid") or !loaded.ok ( - sync.unlock("purchase:" ++ key) return {ok: false} ) object project = loaded.project.assert(object) object found = findGameProduct(project, entry.productId.toStr()) if !found.ok ( - sync.unlock("purchase:" ++ key) return {ok: false} ) object product = found.product.assert(object) @@ -209,7 +208,6 @@ def completeGameProductPurchase(string key) object ( string paymentId = entry.requestId.toStr() if !ownsGameProduct(project.id.toStr(), userId, product.id.toStr()) ( if !saveGameProductEntitlement(project.id.toStr(), userId, product.id.toStr(), paymentId) or !grantGameProductItem(project, product, userId, paymentId) ( - sync.unlock("purchase:" ++ key) return {ok: false, error: "failed to grant game product"} ) // What the buyer paid, which a refund pays back. @@ -218,7 +216,6 @@ def completeGameProductPurchase(string key) object ( addNotification(project.owner.toStr(), buyer, "purchase", {projectId: project.id, projectTitle: project.title, productId: product.id, productName: product.name, amount: paid}) ) forgetPurchase(key) - sync.unlock("purchase:" ++ key) return {ok: true, project: project, product: product} ) diff --git a/src/rotur/rotur-payments.osl b/src/rotur/rotur-payments.osl index ee20d26..252548e 100644 --- a/src/rotur/rotur-payments.osl +++ b/src/rotur/rotur-payments.osl @@ -144,6 +144,7 @@ def startPurchase(string prefix, object entry, string payer, string payee, array // the open request and make two. string lock = "purchase-start:" ++ prefix ++ json.stringify(entry) sync.lock(lock) + defer sync.unlock(lock) object open = loadPurchaseKeys().keys.assertElse(object, {}) array openKeys = open.getKeys() for i openKeys.len ( @@ -152,29 +153,30 @@ def startPurchase(string prefix, object entry, string payer, string payee, array // Only a request still waiting for an answer is reused. A declined one // would leave the person unable to try again until it expired. if samePurchase(stored, entry, prefix, openKey) and roturPaymentStatus(stored.requestId.toStr()) == "pending" ( - sync.unlock(lock) return {ok: true, key: openKey, requestId: stored.requestId.toStr(), approveUrl: stored.approveUrl.toStr(), splits: shares} ) ) string key = prefix ++ randomString(16) object request = requestRoturPayment(payer, payee, shares, amount, note, key, returnUrl) if !request.ok ( - sync.unlock(lock) return request ) entry.requestId = request.id entry.approveUrl = request.approveUrl entry.expiresAt = request.expiresAt entry.at = timestamp + storePurchaseKeyLocked(key, entry) + return {ok: true, key: key, requestId: request.id, approveUrl: request.approveUrl, splits: shares} +) + +def storePurchaseKeyLocked(string key, object entry) ( sync.lock("purchase-keys") + defer sync.unlock("purchase-keys") object store = prunePurchaseKeys(loadPurchaseKeys()) object keys = store.keys keys[key] = entry store.keys = keys savePurchaseKeys(store) - sync.unlock("purchase-keys") - sync.unlock(lock) - return {ok: true, key: key, requestId: request.id, approveUrl: request.approveUrl, splits: shares} ) // The purchase key's entry, once it has been paid, else {}. Rotur is asked if @@ -194,34 +196,40 @@ def paidPurchase(string key) object ( def forgetPurchase(string key) ( sync.lock("purchase-keys") + defer sync.unlock("purchase-keys") object store = loadPurchaseKeys() object keys = store.keys keys.delete(key) store.keys = keys savePurchaseKeys(store) - sync.unlock("purchase-keys") ) // payment.completed: mark the purchase paid, and deliver what needs no more // from the buyer. Donations and refunds wait for the comment or the revoke // that started them. def completeRoturPayment(string reference, string requestId) ( + if !markPurchasePaidLocked(reference, requestId) ( + return + ) + if reference.startsWith("mwbuy_") ( + completeProjectPurchase(reference) + ) else if reference.startsWith("mwgame_") ( + completeGameProductPurchase(reference) + ) +) + +def markPurchasePaidLocked(string reference, string requestId) boolean ( sync.lock("purchase-keys") + defer sync.unlock("purchase-keys") object store = loadPurchaseKeys() object keys = store.keys if !keys.contains(reference) or (keys[reference].requestId ?? "").toStr() != requestId ( - sync.unlock("purchase-keys") - return + return false ) object entry = keys[reference].assert(object) entry.paid = true keys[reference] = entry store.keys = keys savePurchaseKeys(store) - sync.unlock("purchase-keys") - if reference.startsWith("mwbuy_") ( - completeProjectPurchase(reference) - ) else if reference.startsWith("mwgame_") ( - completeGameProductPurchase(reference) - ) + return true ) diff --git a/src/rotur/rotur-webhooks.osl b/src/rotur/rotur-webhooks.osl index 4f6f9b6..21f2285 100644 --- a/src/rotur/rotur-webhooks.osl +++ b/src/rotur/rotur-webhooks.osl @@ -56,10 +56,10 @@ def queueRoturWebhook(object event) boolean ( data = event.data.assert(object) ) sync.lock("rotur-webhooks") + defer sync.unlock("rotur-webhooks") object store = loadRoturWebhooks() object events = store.events.assert(object) if events.contains(id) ( - sync.unlock("rotur-webhooks") return false ) object kept = {} @@ -82,12 +82,12 @@ def queueRoturWebhook(object event) boolean ( } store.events = kept writeFile(roturWebhooksFile, store) - sync.unlock("rotur-webhooks") return true ) def markRoturWebhookDone(string id) ( sync.lock("rotur-webhooks") + defer sync.unlock("rotur-webhooks") object store = loadRoturWebhooks() object events = store.events.assert(object) if events.contains(id) ( @@ -97,7 +97,6 @@ def markRoturWebhookDone(string id) ( store.events = events writeFile(roturWebhooksFile, store) ) - sync.unlock("rotur-webhooks") ) def handleRoturEvent(object event) ( @@ -125,6 +124,7 @@ def handleRoturEvent(object event) ( // Handles every saved event that hasn't been handled yet, one at a time. def processRoturWebhooks() ( sync.lock("rotur-webhooks-worker") + defer sync.unlock("rotur-webhooks-worker") object store = loadRoturWebhooks() object events = store.events.assert(object) array ids = events.getKeys() @@ -136,7 +136,6 @@ def processRoturWebhooks() ( markRoturWebhookDone(id) ) ) - sync.unlock("rotur-webhooks-worker") ) def handleRoturWebhook(*serve.Context c) ( From a4db794e9fce1a677ab3b20766791f75dbcff33c Mon Sep 17 00:00:00 2001 From: Mistium <92952823+Mistium@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:05:32 +0000 Subject: [PATCH 2/7] Save blocked extension URLs Blocking a URL appended it to a copy of the policy's list and never wrote the list back, so the block was never saved. The list is now written back before the policy is saved. --- src/projects/extensions.osl | 19 ++++++++++++------- src/projects/extensions.test.osl | 8 ++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/projects/extensions.osl b/src/projects/extensions.osl index a0fe043..3a8cc81 100644 --- a/src/projects/extensions.osl +++ b/src/projects/extensions.osl @@ -718,6 +718,17 @@ def handleAdminSetExtensionPolicy(*serve.Context c) ( c.json(200, {ok: true, affected: affectedCount}) ) +def setExtensionUrlBlocked(object policy, string extensionUrl, boolean blocked) ( + array blockedUrls = policy.blockedUrls.assert(array) + if blocked and !blockedUrls.contains(extensionUrl) ( + blockedUrls.append(extensionUrl) + ) + if !blocked ( + blockedUrls = removeString(blockedUrls, extensionUrl) + ) + policy.blockedUrls = blockedUrls +) + def handleAdminSetExtensionUrlPolicy(*serve.Context c) ( object body = requestJSON(c) if !body.contains("url") or !body.contains("blocked") ( @@ -735,13 +746,7 @@ def handleAdminSetExtensionUrlPolicy(*serve.Context c) ( return ) object policy = loadExtensionPolicy() - array blockedUrls = policy.blockedUrls.assert(array) - if blocked and !blockedUrls.contains(extensionUrl) ( - blockedUrls.append(extensionUrl) - ) - if !blocked ( - policy.blockedUrls = removeString(blockedUrls, extensionUrl) - ) + setExtensionUrlBlocked(policy, extensionUrl, blocked) if !saveExtensionPolicy(policy) ( c.internalError("could not save URL policy") return diff --git a/src/projects/extensions.test.osl b/src/projects/extensions.test.osl index 727fe4a..0424d01 100644 --- a/src/projects/extensions.test.osl +++ b/src/projects/extensions.test.osl @@ -107,4 +107,12 @@ testing.assert(migrateExtensionUsage() and json.stringify(readExtensionRecord(st array sharedProjects = readExtensionRecord(hash).projects.assert(array) testing.assert(sharedProjects.contains("p2") and sharedProjects.contains("p5"), "the backfill keeps projects the record already listed") +object urlPolicy = loadExtensionPolicy() +setExtensionUrlBlocked(urlPolicy, "https://example.com/bad.js", true) +setExtensionUrlBlocked(urlPolicy, "https://example.com/bad.js", true) +testing.assert(saveExtensionPolicy(urlPolicy) and loadExtensionPolicy().blockedUrls == ["https://example.com/bad.js"], "blocking a URL saves it once") +object reloadedPolicy = loadExtensionPolicy() +setExtensionUrlBlocked(reloadedPolicy, "https://example.com/bad.js", false) +testing.assert(saveExtensionPolicy(reloadedPolicy) and loadExtensionPolicy().blockedUrls.len == 0, "unblocking a URL removes it") + void fs.remove(tmpDir) From 0a8a876aa544638ded675b7282a4b95dd2c56b0b Mon Sep 17 00:00:00 2001 From: Mistium <92952823+Mistium@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:05:38 +0000 Subject: [PATCH 3/7] Release project, git, history and storage locks on every exit path Every remaining lock in these domains is now followed by a deferred unlock, so an error can't leave it held until restart. Where work carried on after the unlock, the locked part moved into its own function, so each lock still covers the same steps: pull numbering, branch changes, pull comment counts and fast-forward merges, the backpack (still taking the quota lock before the backpack lock), extension records, staged assets, the storage ledger queues and the R2 inventory. The remix and reaction wrappers use runLocked. Tests make a locked step throw in pulls, the backpack and the upload quota, and check the lock can be taken again. --- src/git/git-inspection.osl | 5 +- src/git/pulls.osl | 312 +++++++++++++++------------- src/git/pulls.test.osl | 252 ++++++++++++++++++++++ src/history/commit-management.osl | 12 +- src/history/history-migration.osl | 13 +- src/projects/extensions.osl | 143 +++++++------ src/projects/monetize.osl | 3 +- src/projects/project-publishing.osl | 7 +- src/projects/project-quota.osl | 5 +- src/projects/project-quota.test.osl | 107 ++++++++++ src/projects/project-remix.osl | 7 +- src/projects/project-snapshots.osl | 4 +- src/projects/upload-parts.osl | 53 +++-- src/storage/backpack.osl | 103 +++++---- src/storage/backpack.test.osl | 154 ++++++++++++++ src/storage/r2-upload-queue.osl | 6 +- src/storage/r2.osl | 26 +-- src/storage/storage-ledger.osl | 77 ++++--- src/storage/storage-usage.osl | 5 +- src/storage/storage.osl | 2 +- 20 files changed, 939 insertions(+), 357 deletions(-) create mode 100644 src/git/pulls.test.osl create mode 100644 src/projects/project-quota.test.osl create mode 100644 src/storage/backpack.test.osl diff --git a/src/git/git-inspection.osl b/src/git/git-inspection.osl index c7dc3a8..de73d81 100644 --- a/src/git/git-inspection.osl +++ b/src/git/git-inspection.osl @@ -54,24 +54,21 @@ def cachedProjectWorkspacePath(object project) string ( ) string lockName = "git-inspection-workspace-" ++ cacheName sync.lock(lockName) + defer sync.unlock(lockName) if fs.exists(cachedPath) ( - sync.unlock(lockName) return cachedPath ) string temporaryPath = cachedPath ++ ".tmp." ++ randomString(8) if !copyProjectWorkspaceToPath(project, temporaryPath) ( void fs.remove(temporaryPath) - sync.unlock(lockName) return "" ) if !fs.rename(temporaryPath, cachedPath) ( void fs.remove(temporaryPath) if !fs.exists(cachedPath) ( - sync.unlock(lockName) return "" ) ) - sync.unlock(lockName) return cachedPath ) diff --git a/src/git/pulls.osl b/src/git/pulls.osl index 1a646e0..614c3d3 100644 --- a/src/git/pulls.osl +++ b/src/git/pulls.osl @@ -150,8 +150,20 @@ def createNativePull(*serve.Context c, object target, object source, string titl if target.contains("nativePullSeq") and target.nativePullSeq != null ( sequence = target.nativePullSeq.toNum() ) + object inserted = insertNativePullLocked(target, source, title, description, bountyId, baseCommit, sequence, contextString(c, "username"), contextString(c, "userId")) + if inserted.ok != true ( + return inserted + ) + object pull = inserted.pull.assert(object) + sequence = pull.index.toNum() + addNotification(target.owner.toStr(), contextString(c, "username"), "contribution", {projectId: target.id, projectTitle: target.title, pull: sequence}) + return {ok: true, pull: pullResponse(pull)} +) + +def insertNativePullLocked(object target, object source, string title, string description, string bountyId, string baseCommit, number sequence, string author, string authorId) object ( string sequenceLock = "native-pull-sequence:" ++ target.id.toStr() sync.lock(sequenceLock) + defer sync.unlock(sequenceLock) array latestPulls = pullDocs.where("targetProjectId", "==", target.id.toStr()).sort("index", "desc").limit(1).all() sequence = nextPullSequence(sequence, latestPulls) @@ -170,8 +182,8 @@ def createNativePull(*serve.Context c, object target, object source, string titl title: title, body: description.trim(1, 5000), state: "open", - author: contextString(c, "username"), - authorId: contextString(c, "userId"), + author: author, + authorId: authorId, sourceOwner: source.owner, sourceOwnerId: source.ownerId, sourceProjectId: source.id, @@ -189,12 +201,9 @@ def createNativePull(*serve.Context c, object target, object source, string titl bountyId: bountyId } if pullDocs.insertOne(pull).isErr() ( - sync.unlock(sequenceLock) return {ok: false, status: 409, error: "another pull request was created at the same time, try again"} ) - sync.unlock(sequenceLock) - addNotification(target.owner.toStr(), contextString(c, "username"), "contribution", {projectId: target.id, projectTitle: target.title, pull: sequence}) - return {ok: true, pull: pullResponse(pull)} + return {ok: true, pull: pull} ) def handleGetCommits(*serve.Context c) ( @@ -275,19 +284,25 @@ def handleListProjectBranches(*serve.Context c) ( ) def handleProjectBranchMutation(*serve.Context c, string action) ( + object response = mutateProjectBranchesLocked(c, action) + if response.len > 0 ( + c.json(200, response) + ) +) + +def mutateProjectBranchesLocked(*serve.Context c, string action) object ( string lockName = "project-history-" ++ c.param("id") sync.lock(lockName) + defer sync.unlock(lockName) auto res = loadProject(c.param("id")) if !res.ok ( - sync.unlock(lockName) c.notFound("project not found") - return + return {} ) object project = res.project.assert(object) if !canEditProject(project, c) or !canSeeInsideProject(project, c) ( - sync.unlock(lockName) c.forbidden("only project maintainers can manage branches") - return + return {} ) object body = requestJSON(c) string branch = (body.branch ?? "").toStr().strip() @@ -300,9 +315,8 @@ def handleProjectBranchMutation(*serve.Context c, string action) ( string outputPath = tmpDir ++ project.id.toStr() ++ ".branches-output." ++ randomString(8) ++ ".mwp" if !copyProjectWorkspaceToPath(project, sourcePath) ( void fs.remove(sourcePath) - sync.unlock(lockName) c.json(409, {ok: false, error: "save the project before managing its branches", code: "branch_history_unavailable"}) - return + return {} ) object history = projectVersionHistory(project) history.head = projectGitHead(project) @@ -313,24 +327,21 @@ def handleProjectBranchMutation(*serve.Context c, string action) ( if parsedRewrite.isErr() or parsedRewrite.unwrap().assert(object).ok != true or !fs.exists(outputPath) ( string message = if parsedRewrite.isErr() then "could not update the branch" else (parsedRewrite.unwrap().assert(object).error ?? "could not update the branch").toStr() void fs.remove(outputPath) - sync.unlock(lockName) c.json(409, {ok: false, error: message, code: "branch_update_unavailable"}) - return + return {} ) object rewrite = parsedRewrite.unwrap().assert(object) object manifest = rewrite.manifest.assert(object) byte[] workspaceBody = fs.readFileBytes(outputPath).unwrapOr("".toBytes()) void fs.remove(outputPath) if workspaceBody.len > maxWorkspaceBytes ( - sync.unlock(lockName) c.json(413, {ok: false, error: "MistWarp project history exceeds the 512 MB limit", code: "workspace_too_large"}) - return + return {} ) string workspaceKey = projectWorkspaceSnapshotKey(contentHash(workspaceBody)) if !stageR2Blob(workspaceKey, workspaceBody, "application/x-mistwarp-project", "") ( - sync.unlock(lockName) c.internalError("could not store the updated branch history") - return + return {} ) array previousWorkspaceKeys = projectWorkspaceKeys(project) project.workspaceKey = workspaceKey @@ -345,9 +356,8 @@ def handleProjectBranchMutation(*serve.Context c, string action) ( project.edited = timestamp if !saveProject(project) ( removeProjectStorageKeyIfOrphaned(workspaceKey, project.id.toStr(), true) - sync.unlock(lockName) c.internalError("could not save the updated branches") - return + return {} ) for i previousWorkspaceKeys.len ( string previousKey = previousWorkspaceKeys[i].toStr() @@ -360,8 +370,7 @@ def handleProjectBranchMutation(*serve.Context c, string action) ( current: project.gitBranch, branches: projectBranchResponse(project) } - sync.unlock(lockName) - c.json(200, response) + return response ) def handleCreateProjectBranch(*serve.Context c) ( @@ -536,18 +545,7 @@ def handleCreatePullComment(*serve.Context c) ( c.badRequest(created.error.toStr()) return ) - sync.lock("pull-comment-count:" ++ pull["_id"].toStr()) - object latestPull = pullDocument(pull.targetProjectId.toStr(), pull.index.toStr()) - if latestPull.len > 0 ( - number storedCount = storedPullCommentCount(latestPull) - number actualCount = 0 - if storedCount < 0 ( - actualCount = loadComments(pullCommentKey(latestPull)).len - ) - latestPull.commentCount = pullCommentCountAfterCreate(latestPull, actualCount) - putDocument(pullDocs, latestPull["_id"].toStr(), latestPull) - ) - sync.unlock("pull-comment-count:" ++ pull["_id"].toStr()) + countCreatedPullCommentLocked(pull) string commenter = contextString(c, "username") object notificationData = {projectId: project.id, projectTitle: project.title, pull: pull.index, commentId: created.comment.id, preview: created.comment.content.toStr()} if normalizeUsername(pull.author.toStr()) != normalizeUsername(commenter) ( @@ -578,7 +576,14 @@ def handleDeletePullComment(*serve.Context c) ( c.json(deleted.status.toInt(), {ok: false, error: deleted.error}) return ) - sync.lock("pull-comment-count:" ++ pull["_id"].toStr()) + countDeletedPullCommentLocked(pull, deleted.removed.toNum()) + c.json(200, {ok: true}) +) + +def countCreatedPullCommentLocked(object pull) ( + string lockName = "pull-comment-count:" ++ pull["_id"].toStr() + sync.lock(lockName) + defer sync.unlock(lockName) object latestPull = pullDocument(pull.targetProjectId.toStr(), pull.index.toStr()) if latestPull.len > 0 ( number storedCount = storedPullCommentCount(latestPull) @@ -586,11 +591,25 @@ def handleDeletePullComment(*serve.Context c) ( if storedCount < 0 ( actualCount = loadComments(pullCommentKey(latestPull)).len ) - latestPull.commentCount = pullCommentCountAfterDelete(latestPull, deleted.removed.toNum(), actualCount) + latestPull.commentCount = pullCommentCountAfterCreate(latestPull, actualCount) + putDocument(pullDocs, latestPull["_id"].toStr(), latestPull) + ) +) + +def countDeletedPullCommentLocked(object pull, number removed) ( + string lockName = "pull-comment-count:" ++ pull["_id"].toStr() + sync.lock(lockName) + defer sync.unlock(lockName) + object latestPull = pullDocument(pull.targetProjectId.toStr(), pull.index.toStr()) + if latestPull.len > 0 ( + number storedCount = storedPullCommentCount(latestPull) + number actualCount = 0 + if storedCount < 0 ( + actualCount = loadComments(pullCommentKey(latestPull)).len + ) + latestPull.commentCount = pullCommentCountAfterDelete(latestPull, removed, actualCount) putDocument(pullDocs, latestPull["_id"].toStr(), latestPull) ) - sync.unlock("pull-comment-count:" ++ pull["_id"].toStr()) - c.json(200, {ok: true}) ) def handleEditPullComment(*serve.Context c) ( @@ -757,115 +776,13 @@ def handleMergePull(*serve.Context c) ( c.json(409, {ok: false, error: "the pull request history is unavailable", code: "pull_history_unavailable"}) return ) - sync.lock("project-history-" ++ target.id.toStr()) - auto latestTargetResult = loadProject(target.id.toStr()) - auto latestSourceResult = loadProject(source.id.toStr()) - if !latestTargetResult.ok or !latestSourceResult.ok or projectGitHead(latestTargetResult.project.assert(object)) != currentTargetHead or projectGitHead(latestSourceResult.project.assert(object)) != currentSourceHead ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.json(409, {ok: false, error: "a project changed while this merge was processing", code: "head_changed"}) - return - ) - target = latestTargetResult.project.assert(object) - source = latestSourceResult.project.assert(object) - object sourceJsonSnapshot = ensureProjectJsonSnapshot(source) - if !sourceJsonSnapshot.ok ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.internalError("could not prepare the source project") - return - ) - source = sourceJsonSnapshot.project.assert(object) - object sourceWorkspaceSnapshots = ensureProjectWorkspaceSnapshots(source) - if !sourceWorkspaceSnapshots.ok ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.internalError("could not prepare the source project history") + object merged = fastForwardPullLocked(c, pull, target, source, targetWorkspacePath, sourceWorkspacePath, currentTargetHead, currentSourceHead) + if merged.ok != true ( return ) - source = sourceWorkspaceSnapshots.project.assert(object) - string mergeArchivePath = tmpDir ++ target.id.toStr() ++ "." ++ randomString(8) ++ ".pull-merge.mwp" - string targetBranch = if target.contains("gitBranch") and target.gitBranch != null then target.gitBranch.toStr() else "main" - string targetRemixParent = if target.contains("remixParent") and target.remixParent != null then target.remixParent.toStr() else "" - string targetBaseCommit = if target.contains("remixBaseCommit") and target.remixBaseCommit != null then target.remixBaseCommit.toStr() else "" - string targetId = target.id.toStr() - string mergeAuthor = normalizeUsername(contextString(c, "username")) - string mergeMessage = "Merge pull request #" ++ pull.index.toStr() - number mergeTimestamp = timestamp - string mergeArchiveJson = gitinspection.CreateFastForwardMergeArchiveJSON(targetWorkspacePath, currentTargetHead, sourceWorkspacePath, currentSourceHead, mergeArchivePath, targetId, targetRemixParent, targetBaseCommit, targetBranch, mergeAuthor, mergeMessage, mergeTimestamp / 1000) - auto parsedMergeArchive = json.parse(mergeArchiveJson, {type: "object"}) - if parsedMergeArchive.isErr() or parsedMergeArchive.unwrap().assert(object).ok != true or !fs.exists(mergeArchivePath) ( - void fs.remove(mergeArchivePath) - sync.unlock("project-history-" ++ target.id.toStr()) - c.internalError("could not create the pull request merge commit") - return - ) - object mergeArchiveResult = parsedMergeArchive.unwrap().assert(object) - object mergeArchiveGit = mergeArchiveResult.manifest.assert(object) - byte[] mergeArchiveBody = fs.readFileBytes(mergeArchivePath).unwrapOr("".toBytes()) - void fs.remove(mergeArchivePath) - object checkedHistory = validateUploadedHistory(target, mergeArchiveBody, json.stringify(mergeArchiveGit), currentTargetHead) - if !checkedHistory.ok ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.json(409, {ok: false, code: "invalid_workspace", error: checkedHistory.error}) - return - ) - mergeArchiveBody = checkedHistory.data.assert(byte[]) - mergeArchiveGit = checkedHistory.manifest.assert(object) - if mergeArchiveBody.len > maxWorkspaceBytes ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.json(413, {ok: false, error: "MistWarp project history exceeds the 90 MB limit", code: "workspace_too_large"}) - return - ) - string mergeArchiveKey = projectWorkspaceSnapshotKey(contentHash(mergeArchiveBody)) - if !stageR2Blob(mergeArchiveKey, mergeArchiveBody, "application/x-mistwarp-project", "") ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.internalError("could not store the pull request merge history") - return - ) - string previousJsonKey = target.jsonKey.toStr() - array previousWorkspaceKeys = projectWorkspaceKeys(target) - array previousExtensions = storedProjectExtensions(target) - target.jsonKey = source.jsonKey - target.jsonGzip = true - target.inheritedJson = true - target.pendingJson = false - target.workspaceKey = mergeArchiveKey - target.workspaceLayers = [mergeArchiveKey] - target.historyBytes = mergeArchiveBody.len - target.gitHead = mergeArchiveGit.head - target.gitBranch = mergeArchiveGit.branch - target.gitCommits = mergeArchiveGit.commits - target.gitGraph = mergeArchiveGit.graph - target.assets = source.assets - target.sizeBytes = projectSizeBytes(source) - target.edited = timestamp - if source.contains("jsonBytes") ( - target.jsonBytes = source.jsonBytes - ) - if source.contains("storedJsonBytes") ( - target.storedJsonBytes = source.storedJsonBytes - ) - if source.contains("assetBytes") ( - target.assetBytes = source.assetBytes - ) - if source.contains("extensions") ( - target.extensions = source.extensions - ) else ( - target.extensions = [] - ) - target.extensionsIndexed = source.contains("extensionsIndexed") and source.extensionsIndexed == true - if !storeProjectExtensions(target.id.toStr(), previousExtensions, storedProjectExtensions(target), {}) or !saveProject(target) ( - removeProjectStorageKeyIfOrphaned(mergeArchiveKey, target.id.toStr(), true) - sync.unlock("project-history-" ++ target.id.toStr()) - c.internalError("could not save the merged project") - return - ) - upsertIndexEntry(target) - storageSettleProject(target) - if !completeNativePull(target, pull.index.toStr(), currentTargetHead, contextString(c, "username")) ( - sync.unlock("project-history-" ++ target.id.toStr()) - c.internalError("the project merged, but the pull request could not be closed") - return - ) - sync.unlock("project-history-" ++ target.id.toStr()) + target = merged.target.assert(object) + string previousJsonKey = merged.previousJsonKey.toStr() + array previousWorkspaceKeys = merged.previousWorkspaceKeys.assert(array) if previousJsonKey != target.jsonKey.toStr() ( removeProjectStorageKeyIfOrphaned(previousJsonKey, target.id.toStr(), false) ) @@ -904,6 +821,111 @@ def handleMergePull(*serve.Context c) ( }) ) +def fastForwardPullLocked(*serve.Context c, object pull, object target, object source, string targetWorkspacePath, string sourceWorkspacePath, string currentTargetHead, string currentSourceHead) object ( + string lockName = "project-history-" ++ target.id.toStr() + sync.lock(lockName) + defer sync.unlock(lockName) + auto latestTargetResult = loadProject(target.id.toStr()) + auto latestSourceResult = loadProject(source.id.toStr()) + if !latestTargetResult.ok or !latestSourceResult.ok or projectGitHead(latestTargetResult.project.assert(object)) != currentTargetHead or projectGitHead(latestSourceResult.project.assert(object)) != currentSourceHead ( + c.json(409, {ok: false, error: "a project changed while this merge was processing", code: "head_changed"}) + return {ok: false} + ) + target = latestTargetResult.project.assert(object) + source = latestSourceResult.project.assert(object) + object sourceJsonSnapshot = ensureProjectJsonSnapshot(source) + if !sourceJsonSnapshot.ok ( + c.internalError("could not prepare the source project") + return {ok: false} + ) + source = sourceJsonSnapshot.project.assert(object) + object sourceWorkspaceSnapshots = ensureProjectWorkspaceSnapshots(source) + if !sourceWorkspaceSnapshots.ok ( + c.internalError("could not prepare the source project history") + return {ok: false} + ) + source = sourceWorkspaceSnapshots.project.assert(object) + string mergeArchivePath = tmpDir ++ target.id.toStr() ++ "." ++ randomString(8) ++ ".pull-merge.mwp" + string targetBranch = if target.contains("gitBranch") and target.gitBranch != null then target.gitBranch.toStr() else "main" + string targetRemixParent = if target.contains("remixParent") and target.remixParent != null then target.remixParent.toStr() else "" + string targetBaseCommit = if target.contains("remixBaseCommit") and target.remixBaseCommit != null then target.remixBaseCommit.toStr() else "" + string targetId = target.id.toStr() + string mergeAuthor = normalizeUsername(contextString(c, "username")) + string mergeMessage = "Merge pull request #" ++ pull.index.toStr() + number mergeTimestamp = timestamp + string mergeArchiveJson = gitinspection.CreateFastForwardMergeArchiveJSON(targetWorkspacePath, currentTargetHead, sourceWorkspacePath, currentSourceHead, mergeArchivePath, targetId, targetRemixParent, targetBaseCommit, targetBranch, mergeAuthor, mergeMessage, mergeTimestamp / 1000) + auto parsedMergeArchive = json.parse(mergeArchiveJson, {type: "object"}) + if parsedMergeArchive.isErr() or parsedMergeArchive.unwrap().assert(object).ok != true or !fs.exists(mergeArchivePath) ( + void fs.remove(mergeArchivePath) + c.internalError("could not create the pull request merge commit") + return {ok: false} + ) + object mergeArchiveResult = parsedMergeArchive.unwrap().assert(object) + object mergeArchiveGit = mergeArchiveResult.manifest.assert(object) + byte[] mergeArchiveBody = fs.readFileBytes(mergeArchivePath).unwrapOr("".toBytes()) + void fs.remove(mergeArchivePath) + object checkedHistory = validateUploadedHistory(target, mergeArchiveBody, json.stringify(mergeArchiveGit), currentTargetHead) + if !checkedHistory.ok ( + c.json(409, {ok: false, code: "invalid_workspace", error: checkedHistory.error}) + return {ok: false} + ) + mergeArchiveBody = checkedHistory.data.assert(byte[]) + mergeArchiveGit = checkedHistory.manifest.assert(object) + if mergeArchiveBody.len > maxWorkspaceBytes ( + c.json(413, {ok: false, error: "MistWarp project history exceeds the 90 MB limit", code: "workspace_too_large"}) + return {ok: false} + ) + string mergeArchiveKey = projectWorkspaceSnapshotKey(contentHash(mergeArchiveBody)) + if !stageR2Blob(mergeArchiveKey, mergeArchiveBody, "application/x-mistwarp-project", "") ( + c.internalError("could not store the pull request merge history") + return {ok: false} + ) + string previousJsonKey = target.jsonKey.toStr() + array previousWorkspaceKeys = projectWorkspaceKeys(target) + array previousExtensions = storedProjectExtensions(target) + target.jsonKey = source.jsonKey + target.jsonGzip = true + target.inheritedJson = true + target.pendingJson = false + target.workspaceKey = mergeArchiveKey + target.workspaceLayers = [mergeArchiveKey] + target.historyBytes = mergeArchiveBody.len + target.gitHead = mergeArchiveGit.head + target.gitBranch = mergeArchiveGit.branch + target.gitCommits = mergeArchiveGit.commits + target.gitGraph = mergeArchiveGit.graph + target.assets = source.assets + target.sizeBytes = projectSizeBytes(source) + target.edited = timestamp + if source.contains("jsonBytes") ( + target.jsonBytes = source.jsonBytes + ) + if source.contains("storedJsonBytes") ( + target.storedJsonBytes = source.storedJsonBytes + ) + if source.contains("assetBytes") ( + target.assetBytes = source.assetBytes + ) + if source.contains("extensions") ( + target.extensions = source.extensions + ) else ( + target.extensions = [] + ) + target.extensionsIndexed = source.contains("extensionsIndexed") and source.extensionsIndexed == true + if !storeProjectExtensions(target.id.toStr(), previousExtensions, storedProjectExtensions(target), {}) or !saveProject(target) ( + removeProjectStorageKeyIfOrphaned(mergeArchiveKey, target.id.toStr(), true) + c.internalError("could not save the merged project") + return {ok: false} + ) + upsertIndexEntry(target) + storageSettleProject(target) + if !completeNativePull(target, pull.index.toStr(), currentTargetHead, contextString(c, "username")) ( + c.internalError("the project merged, but the pull request could not be closed") + return {ok: false} + ) + return {ok: true, target: target, previousJsonKey: previousJsonKey, previousWorkspaceKeys: previousWorkspaceKeys} +) + def handleClosePull(*serve.Context c) ( object pull = pullDocument(c.param("id"), c.param("index")) if pull.len == 0 or pull.state.toStr() != "open" ( diff --git a/src/git/pulls.test.osl b/src/git/pulls.test.osl new file mode 100644 index 0000000..741365b --- /dev/null +++ b/src/git/pulls.test.osl @@ -0,0 +1,252 @@ +import "std:testing" +import "std:db" +import "std:fs" +import "std:json" +import "std:requests" +import "std:serve" +import "std:sync" +import "std:thread" +import "go:mistwarp.local/gitinspection" + +string dbPath = "/tmp/mistwarp-pulls-test-" ++ timestamp.toStr() ++ ".db" +auto testDb = db.open(dbPath) +*db.Collection pullDocs = testDb.collection("project_pulls") +string tmpDir = "/tmp/mistwarp-pulls-test-" ++ timestamp.toStr() ++ "/" +number maxWorkspaceBytes = 1000 +string appURL = "http://127.0.0.1" + +boolean failing = false +def failWhenAsked(string what) ( + if failing ( + throw "IOError: " ++ what ++ " failed" + ) +) + +def lockIsFree(string lockName) boolean ( + boolean free = thread.new(def() -> ( + boolean taken = sync.tryLock(lockName) + if taken ( + sync.unlock(lockName) + ) + return taken + )).wait() + return free +) + +def canonicalHistoryHead(object project) string ( + return (project.gitHead ?? "").toStr() +) +def storedPullCommentCount(object pull) number ( + return if pull.contains("commentCount") then pull.commentCount.toNum() else -1 +) +def pullCommentCountAfterCreate(object pull, number actualCount) number ( + return actualCount + 1 +) +def pullCommentCountAfterDelete(object pull, number removed, number actualCount) number ( + return actualCount +) +def nextPullSequence(number legacySequence, array latestPulls) number ( + failWhenAsked("the pull sequence") + return legacySequence + 1 +) +def loadComments(string key) array ( + failWhenAsked("loading comments") + return [] +) +def loadProject(string id) object ( + failWhenAsked("loading the project") + return {ok: false, missing: true} +) +def putDocument(*db.Collection docs, string id, object doc) boolean ( + void docs.deleteOne({_id: id}) + return docs.insertOne(doc).isOk() +) +def projectWorkspaceKeys(object project) array ( + return [] +) +def contextBool(*serve.Context c, string key) boolean ( + return false +) +def contextString(*serve.Context c, string key) string ( + return "" +) +def addNotification(string target, string actor, string kind, object payload) ( + return +) + +def canEditProject(object project,*serve.Context c) boolean ( + return false +) +def canSeeInsideProject(object project,*serve.Context c) boolean ( + return false +) +def canViewProject(object project,*serve.Context c) boolean ( + return false +) +def graphBranchLog(object graph, string branch) array ( + return [] +) +def identityMatches(string storedId, string storedUsername, string userId, string username) boolean ( + return false +) +def projectHiddenFrom(object project,*serve.Context c) boolean ( + return false +) +def projectRole(object project, string username) string ( + return "" +) +def projectVersionHistory(object project) object ( + return {} +) +def projectWorkspaceFingerprint(object project) string ( + return "" +) +def requestJSON(*serve.Context c) object ( + return {} +) +def validGitObjectId(string oid) boolean ( + return false +) +def cachedProjectWorkspacePath(object project) string ( + return "" +) +def commentResponse(object comment, string username) object ( + return {} +) +def copyProjectWorkspaceToPath(object project, string outputPath) boolean ( + return false +) +def createComment(string key, string author, string authorId, string content, string parentId, string kind, number donationAmount, string donationPaymentId) object ( + return {} +) +def deleteComment(string key, string cid, string requester, string moderator, boolean isAdmin) object ( + return {} +) +def normalizeUsername(string username) string ( + return "" +) +def notifyMentions(string content, string actor, array skip, object extra) ( + return +) +def paginate(array items, number offset, number limit) array ( + return [] +) +def projectWorkspaceSnapshotKey(string hash) string ( + return "" +) +def randomString(number length) string ( + return "" +) +def removeProjectStorageKeyIfOrphaned(string key, string excludedId, boolean workspace) ( + return +) +def saveProject(object project) boolean ( + return false +) +def sourcePolicyAllowsViewer(object project,*serve.Context c) boolean ( + return false +) +def stageR2Blob(string key, byte[] body, string contentType, string contentEncoding) boolean ( + return false +) +def usersAreBlocked(string first, string second) boolean ( + return false +) +def awardContributionBounty(string bountyId, string projectId, string recipient) boolean ( + return false +) +def contentHash(byte[] body) string ( + return "" +) +def editComment(string key, string cid, string requester, string content) object ( + return {} +) +def ensureProjectJsonSnapshot(object project) object ( + return {} +) +def ensureProjectWorkspaceSnapshots(object project) object ( + return {} +) +def projectRecordsCommit(object project, string sha) boolean ( + return false +) +def projectSizeBytes(object project) number ( + return 0 +) +def storageSettleProject(object project) ( + return +) +def storedProjectExtensions(object project) array ( + return [] +) +def storeProjectExtensions(string projectId, array previousExtensions, array extensions, object sources) boolean ( + return false +) +def upsertIndexEntry(object project) boolean ( + return false +) +def validateUploadedHistory(object project, byte[] body, string metadata, string currentHead) object ( + return {} +) +import "./pulls.osl" + +object made = insertNativePullLocked({id: "p1"}, {id: "p2"}, "Pull", "Body", "", "base", 0, "sam", "u_sam") +testing.assert(made.ok == true and made.pull.index == 1 and lockIsFree("native-pull-sequence:p1"), "a pull is numbered and the sequence lock released") +failing = true +testing.assert(try(insertNativePullLocked({id: "p1"}, {id: "p2"}, "Pull", "Body", "", "base", 0, "sam", "u_sam")).isErr(), "a failure while numbering a pull reaches the caller") +testing.assert(lockIsFree("native-pull-sequence:p1"), "the sequence lock is released when numbering throws") +failing = false + +object legacyPull = {"_id": "p1:1", targetProjectId: "p1", index: 1} +void putDocument(pullDocs, "p1:1", legacyPull) +countCreatedPullCommentLocked(legacyPull) +testing.assert(pullDocument("p1", "1").commentCount == 1 and lockIsFree("pull-comment-count:p1:1"), "a new comment is counted") +void putDocument(pullDocs, "p1:1", legacyPull) +def countCreated() boolean ( + countCreatedPullCommentLocked(legacyPull) + return true +) +def countDeleted() boolean ( + countDeletedPullCommentLocked(legacyPull, 1) + return true +) +failing = true +testing.assert(try(countCreated()).isErr() and lockIsFree("pull-comment-count:p1:1"), "the comment count lock is released when counting a new comment throws") +testing.assert(try(countDeleted()).isErr() and lockIsFree("pull-comment-count:p1:1"), "the comment count lock is released when counting a deleted comment throws") +failing = false + +def handleTestMerge(*serve.Context c) ( + void fastForwardPullLocked(c, {index: 1}, {id: "p1"}, {id: "p2"}, "", "", "h1", "h2") +) +def handleTestBranches(*serve.Context c) ( + handleProjectBranchMutation(c, "create") +) +*serve.Router app = serve.new() +app.POST("/projects/:id/merge", handleTestMerge) +app.POST("/projects/:id/branches", handleTestBranches) +app.GET("/ready", def(*serve.Context c) -> ( + c.json(200, {ok: true}) +)) +def runApp() ( + app.run(":18353") +) +thread.new(runApp) +string base = "http://127.0.0.1:18353" +for i 50 ( + if requests.get(base ++ "/ready").isOk() ( + break + ) + wait 0.1 +) + +failing = true +number mergeStatus = requests.post(base ++ "/projects/p1/merge", {body: "{}"}).unwrapOr({status: 0}).status.toNum() +testing.assert(mergeStatus >= 500 and lockIsFree("project-history-p1"), "the save lock is released when a pull merge throws") +number branchStatus = requests.post(base ++ "/projects/p1/branches", {body: "{}"}).unwrapOr({status: 0}).status.toNum() +testing.assert(branchStatus >= 500 and lockIsFree("project-history-p1"), "the save lock is released when a branch change throws") +failing = false +number missingStatus = requests.post(base ++ "/projects/p1/branches", {body: "{}"}).unwrapOr({status: 0}).status.toNum() +testing.assert(missingStatus == 404 and lockIsFree("project-history-p1"), "branch changes still answer once the lock is free again") + +testDb.close() +void fs.remove(dbPath) diff --git a/src/history/commit-management.osl b/src/history/commit-management.osl index 9ab7709..d0e37e7 100644 --- a/src/history/commit-management.osl +++ b/src/history/commit-management.osl @@ -92,10 +92,7 @@ def applyCommitPatch(string id, string sha, object body, string username, boolea ) def handlePatchCommit(*serve.Context c) ( - string lockName = "project-history-" ++ c.param("id") - sync.lock(lockName) - object res = applyCommitPatch(c.param("id"), c.param("sha"), requestJSON(c), contextString(c, "username"), contextBool(c, "isAdmin")) - sync.unlock(lockName) + object res = applyCommitPatchLocked(c) if res.status.toInt() == 404 ( c.notFound(res.error.toStr()) ) else if res.status.toInt() == 403 ( @@ -109,6 +106,13 @@ def handlePatchCommit(*serve.Context c) ( ) ) +def applyCommitPatchLocked(*serve.Context c) object ( + string lockName = "project-history-" ++ c.param("id") + sync.lock(lockName) + defer sync.unlock(lockName) + return applyCommitPatch(c.param("id"), c.param("sha"), requestJSON(c), contextString(c, "username"), contextBool(c, "isAdmin")) +) + def handleGetCommitCollaborators(*serve.Context c) ( c.setHeader("Cache-Control", "private, no-store") c.setHeader("CDN-Cache-Control", "private, no-store") diff --git a/src/history/history-migration.osl b/src/history/history-migration.osl index bf9e9e0..db68d08 100644 --- a/src/history/history-migration.osl +++ b/src/history/history-migration.osl @@ -28,10 +28,10 @@ def copyHistoryProjectJsonToPath(object project, string outputPath) boolean ( string cachePath = cacheDir ++ project.jsonKey.toStr().hashSHA256() ++ ".json.gz" string lockName = "history-migration-json-" ++ project.jsonKey.toStr().hashSHA256() sync.lock(lockName) + defer sync.unlock(lockName) void fs.mkdir(cacheDir, {recursive: true}) boolean ready = fs.exists(cachePath) or copyStoredProjectJsonToPath(project, cachePath) boolean copied = ready and copyFileReplacing(cachePath, outputPath) - sync.unlock(lockName) return copied ) @@ -40,10 +40,10 @@ def copyHistoryAssetToPath(string name, string outputPath) boolean ( string cachePath = cacheDir ++ name string lockName = "history-migration-asset-" ++ name sync.lock(lockName) + defer sync.unlock(lockName) void fs.mkdir(cacheDir, {recursive: true}) boolean ready = fs.exists(cachePath) or copyStorageKeyToPath(assetObjectKey(name), cachePath) boolean copied = ready and copyFileReplacing(cachePath, outputPath) - sync.unlock(lockName) return copied ) @@ -258,7 +258,12 @@ def missingProjectHistoryFiles() array ( ) def logHistoryMigrationProgress(object stats, string name, object res) ( + log countHistoryMigrationLocked(stats, name, res) +) + +def countHistoryMigrationLocked(object stats, string name, object res) string ( sync.lock("history-migration-progress") + defer sync.unlock("history-migration-progress") stats.processed = stats.processed.toNum() + 1 string status = "skipped" if !res.ok ( @@ -270,9 +275,7 @@ def logHistoryMigrationProgress(object stats, string name, object res) ( ) else ( stats.skipped = stats.skipped.toNum() + 1 ) - string message = "MistWarp history migration " ++ stats.processed.toStr() ++ "/" ++ stats.total.toStr() ++ " (generated " ++ stats.generated.toStr() ++ ", skipped " ++ stats.skipped.toStr() ++ ", failed " ++ stats.failed.toStr() ++ "): " ++ name ++ " " ++ status - sync.unlock("history-migration-progress") - log message + return "MistWarp history migration " ++ stats.processed.toStr() ++ "/" ++ stats.total.toStr() ++ " (generated " ++ stats.generated.toStr() ++ ", skipped " ++ stats.skipped.toStr() ++ ", failed " ++ stats.failed.toStr() ++ "): " ++ name ++ " " ++ status ) def generateMissingProjectHistoriesWorker(number workerIndex, number workerCount, array files, object stats,*sync.WaitGroup jobs) ( diff --git a/src/projects/extensions.osl b/src/projects/extensions.osl index 3a8cc81..0ffd1c3 100644 --- a/src/projects/extensions.osl +++ b/src/projects/extensions.osl @@ -281,23 +281,29 @@ def removeProjectExtensionUsage(string projectId, array extensions) boolean ( return false ) if !seen.contains(hash) ( - string lockName = "extension-" ++ hash - sync.lock(lockName) - object record = extensionRecord(policy, hash) - if record.contains("projects") ( - record.projects = removeString(record.projects.assert(array), projectId) - if !writeFile(extensionRecordPath(hash), record) ( - sync.unlock(lockName) - return false - ) + if !removeExtensionProjectLocked(policy, hash, projectId) ( + return false ) - sync.unlock(lockName) seen[hash] = true ) ) return true ) +def removeExtensionProjectLocked(object policy, string hash, string projectId) boolean ( + string lockName = "extension-" ++ hash + sync.lock(lockName) + defer sync.unlock(lockName) + object record = extensionRecord(policy, hash) + if record.contains("projects") ( + record.projects = removeString(record.projects.assert(array), projectId) + if !writeFile(extensionRecordPath(hash), record) ( + return false + ) + ) + return true +) + def storeProjectExtensions(string projectId, array previousExtensions, array extensions, object sources) boolean ( if !removeProjectExtensionUsage(projectId, previousExtensions) ( return false @@ -312,47 +318,49 @@ def storeProjectExtensions(string projectId, array previousExtensions, array ext string extensionUrl = extension.url.toStr() string reviewUrl = extensionReviewUrl(extensionUrl) boolean gallery = isGalleryExtensionUrl(extensionUrl) - string lockName = "extension-" ++ hash - sync.lock(lockName) - if !gallery and !fs.exists(extensionSourcePath(hash)) and sources.contains(extensionUrl) ( - if !fs.writeFile(extensionSourcePath(hash), sources[extensionUrl].toStr()) ( - sync.unlock(lockName) - return false - ) - ) - object record = extensionRecord(policy, hash) - if record.getKeys().len == 0 ( - record = {status: "untrusted", urls: [], projects: [], firstSeen: timestamp} - ) - if !record.contains("urls") ( - record.urls = [] - ) - if !record.contains("projects") ( - record.projects = [] - ) - if gallery ( - record.status = "trusted" - record.gallery = true - ) - array recordUrls = record.urls.assert(array) - array projects = record.projects.assert(array) - if !recordUrls.contains(reviewUrl) ( - recordUrls.append(reviewUrl) - ) - if !projects.contains(projectId) ( - projects.append(projectId) - ) - record.urls = recordUrls - record.projects = projects - if !writeFile(extensionRecordPath(hash), record) ( - sync.unlock(lockName) + if !recordProjectExtensionLocked(policy, hash, projectId, extensionUrl, reviewUrl, gallery, sources) ( return false ) - sync.unlock(lockName) ) return true ) +def recordProjectExtensionLocked(object policy, string hash, string projectId, string extensionUrl, string reviewUrl, boolean gallery, object sources) boolean ( + string lockName = "extension-" ++ hash + sync.lock(lockName) + defer sync.unlock(lockName) + if !gallery and !fs.exists(extensionSourcePath(hash)) and sources.contains(extensionUrl) ( + if !fs.writeFile(extensionSourcePath(hash), sources[extensionUrl].toStr()) ( + return false + ) + ) + object record = extensionRecord(policy, hash) + if record.getKeys().len == 0 ( + record = {status: "untrusted", urls: [], projects: [], firstSeen: timestamp} + ) + if !record.contains("urls") ( + record.urls = [] + ) + if !record.contains("projects") ( + record.projects = [] + ) + if gallery ( + record.status = "trusted" + record.gallery = true + ) + array recordUrls = record.urls.assert(array) + array projects = record.projects.assert(array) + if !recordUrls.contains(reviewUrl) ( + recordUrls.append(reviewUrl) + ) + if !projects.contains(projectId) ( + projects.append(projectId) + ) + record.urls = recordUrls + record.projects = projects + return writeFile(extensionRecordPath(hash), record) +) + def storedProjectExtensions(object project) array ( if project.contains("extensions") and project.extensions != null ( return project.extensions.assert(array) @@ -378,9 +386,9 @@ def migrateExtensionUsage() boolean ( return true ) sync.lock("extension-index-migration") + defer sync.unlock("extension-index-migration") state = readFile(statePath) if extensionUsageMigrated(state) ( - sync.unlock("extension-index-migration") return true ) array files = fs.readDir(projectsDir).unwrapOr([]) @@ -390,13 +398,11 @@ def migrateExtensionUsage() boolean ( object project = res.data.assert(object) array extensions = storedProjectExtensions(project) if extensions.len > 0 and !storeProjectExtensions(project.id.toStr(), [], extensions, {}) ( - sync.unlock("extension-index-migration") return false ) ) ) boolean saved = writeFile(statePath, {version: 2}) - sync.unlock("extension-index-migration") return saved ) @@ -578,11 +584,7 @@ def ensureExtensionRecords(object policy) boolean ( if !record.contains("projects") ( record.projects = [] ) - string lockName = "extension-" ++ hash - sync.lock(lockName) - boolean saved = fs.exists(extensionRecordPath(hash)) or writeFile(extensionRecordPath(hash), record) - sync.unlock(lockName) - if !saved ( + if !writeMissingExtensionRecordLocked(hash, record) ( return false ) ) @@ -593,11 +595,7 @@ def ensureExtensionRecords(object policy) boolean ( if fileName.endsWith(".js") ( string hash = fileName.split(".")[1] if isExtensionHash(hash) and !fs.exists(extensionRecordPath(hash)) ( - string lockName = "extension-" ++ hash - sync.lock(lockName) - boolean saved = fs.exists(extensionRecordPath(hash)) or writeFile(extensionRecordPath(hash), {status: "untrusted", urls: [], projects: [], firstSeen: timestamp}) - sync.unlock(lockName) - if !saved ( + if !writeMissingExtensionRecordLocked(hash, {status: "untrusted", urls: [], projects: [], firstSeen: timestamp}) ( return false ) ) @@ -606,6 +604,13 @@ def ensureExtensionRecords(object policy) boolean ( return true ) +def writeMissingExtensionRecordLocked(string hash, object record) boolean ( + string lockName = "extension-" ++ hash + sync.lock(lockName) + defer sync.unlock(lockName) + return fs.exists(extensionRecordPath(hash)) or writeFile(extensionRecordPath(hash), record) +) + def handleAdminGetExtensions(*serve.Context c) ( object policy = loadExtensionPolicy() if !ensureExtensionRecords(policy) ( @@ -696,16 +701,9 @@ def handleAdminSetExtensionPolicy(*serve.Context c) ( c.badRequest("gallery extensions are always trusted") return ) - string lockName = "extension-" ++ hash - sync.lock(lockName) - record = readExtensionRecord(hash) - record.status = status - if !writeFile(extensionRecordPath(hash), record) ( - sync.unlock(lockName) - c.internalError("could not save extension policy") + if !saveExtensionStatusLocked(c, hash, status) ( return ) - sync.unlock(lockName) number affectedCount = 0 if status == "blocked" ( object affected = unshareProjectsUsingHash(hash) @@ -718,6 +716,19 @@ def handleAdminSetExtensionPolicy(*serve.Context c) ( c.json(200, {ok: true, affected: affectedCount}) ) +def saveExtensionStatusLocked(*serve.Context c, string hash, string status) boolean ( + string lockName = "extension-" ++ hash + sync.lock(lockName) + defer sync.unlock(lockName) + object record = readExtensionRecord(hash) + record.status = status + if !writeFile(extensionRecordPath(hash), record) ( + c.internalError("could not save extension policy") + return false + ) + return true +) + def setExtensionUrlBlocked(object policy, string extensionUrl, boolean blocked) ( array blockedUrls = policy.blockedUrls.assert(array) if blocked and !blockedUrls.contains(extensionUrl) ( diff --git a/src/projects/monetize.osl b/src/projects/monetize.osl index 6126078..6381cdc 100644 --- a/src/projects/monetize.osl +++ b/src/projects/monetize.osl @@ -460,10 +460,10 @@ def handleProjectSalesEnded(*serve.Context c) ( def completeProjectPurchase(string key) object ( sync.lock("purchase:" ++ key) + defer sync.unlock("purchase:" ++ key) object entry = paidPurchase(key) auto res = loadProject((entry.projectId ?? "").toStr()) if !entry.contains("paid") or !res.ok ( - sync.unlock("purchase:" ++ key) return {ok: false} ) object project = res.project.assert(object) @@ -484,6 +484,5 @@ def completeProjectPurchase(string key) object ( addNotification(seller, buyer, "purchase", {projectId: project.id, projectTitle: project.title, amount: owed}) ) forgetPurchase(key) - sync.unlock("purchase:" ++ key) return {ok: true, project: project} ) diff --git a/src/projects/project-publishing.osl b/src/projects/project-publishing.osl index c9c8ff3..12b0e64 100644 --- a/src/projects/project-publishing.osl +++ b/src/projects/project-publishing.osl @@ -134,10 +134,9 @@ def handleViewProject(*serve.Context c) ( ) def handleReactProject(*serve.Context c) ( - string lockName = "project-reactions:" ++ c.param("id") - sync.lock(lockName) - handleReactProjectLocked(c) - sync.unlock(lockName) + runLocked("project-reactions:" ++ c.param("id"), def() -> ( + handleReactProjectLocked(c) + )) ) def handleReactProjectLocked(*serve.Context c) ( diff --git a/src/projects/project-quota.osl b/src/projects/project-quota.osl index 267fa66..169254c 100644 --- a/src/projects/project-quota.osl +++ b/src/projects/project-quota.osl @@ -142,15 +142,14 @@ def storageFullResponse(*serve.Context c, object details, number neededBytes) ( def reservePendingUpload(string username, string name, number bytes, number limit, boolean enforceLimit) object ( string lockName = "upload-quota-" ++ normalizeUsername(username) sync.lock(lockName) + defer sync.unlock(lockName) array events = pendingUploadEvents(username) number used = ownerStorageBytes(username) + quotaTotalBytes(events, timestamp, quotaWindowMs) if enforceLimit and used + bytes > limit ( - sync.unlock(lockName) return {ok: false, used: used} ) events.append({t: timestamp, bytes: bytes, name: name}) writeFile(quotaPath(username), {pending: events}) - sync.unlock(lockName) return {ok: true, used: used} ) @@ -160,11 +159,11 @@ def releasePendingUploads(string username, array names) ( ) string lockName = "upload-quota-" ++ normalizeUsername(username) sync.lock(lockName) + defer sync.unlock(lockName) array events = pendingUploadEvents(username) if events.len > 0 ( writeFile(quotaPath(username), {pending: quotaWithoutNames(events, names, timestamp, quotaWindowMs)}) ) - sync.unlock(lockName) ) // The most a student's project may occupy after a save, or -1 when the diff --git a/src/projects/project-quota.test.osl b/src/projects/project-quota.test.osl new file mode 100644 index 0000000..006f93f --- /dev/null +++ b/src/projects/project-quota.test.osl @@ -0,0 +1,107 @@ +import "std:testing" +import "std:fs" +import "std:json" +import "std:serve" +import "std:sync" +import "std:thread" +import "../storage/quota-policy.osl" + +string quotaDir = "/tmp/mistwarp-quota-test/" +string trashDir = "/tmp/mistwarp-quota-test-trash/" +number quotaWindowMs = 3600000 +string roturBase = "http://127.0.0.1:9" + +boolean failing = false +object written = {} +def readFile(string path) object ( + if failing ( + throw "IOError: disk gone" + ) + if written.contains(path) ( + return {ok: true, data: written[path]} + ) + return {ok: false} +) +def writeFile(string path, object data) boolean ( + written[path] = data + return true +) +def normalizeUsername(string username) string ( + return username.toLower().strip() +) + +def backpackStoredBytes(string userId) number ( + return 0 +) +def contextBool(*serve.Context c, string key) boolean ( + return false +) +def contextString(*serve.Context c, string key) string ( + return "" +) +def identityMatches(string storedId, string storedUsername, string userId, string username) boolean ( + return false +) +def ledgerAllowance(string userId) object ( + return {} +) +def mistwarpBenefitsForProject(object project) object ( + return {} +) +def mistwarpBenefitsForUsername(string username) object ( + return {} +) +def ownerIndexEntries(string owner, boolean includeUnshared) array ( + return [] +) +def paginate(array items, number offset, number limit) array ( + return [] +) +def projectOwnerId(object project) string ( + return "" +) +def projectStorageBytes(object project) number ( + return 0 +) +def userIdForUsername(string username) string ( + return "" +) +def usesRoturStorage(string userId) boolean ( + return false +) +def mistwarpBenefitsForUserId(string userId) object ( + return {} +) +def pendingAssetHold(string ownerId, string username, string name, number bytes, boolean enforce) object ( + return {} +) +def releasePendingAssetHolds(string ownerId, string username, array names) ( + return +) +def lockIsFree(string lockName) boolean ( + boolean free = thread.new(def() -> ( + boolean taken = sync.tryLock(lockName) + if taken ( + sync.unlock(lockName) + ) + return taken + )).wait() + return free +) + +import "./project-quota.osl" + +object reserved = reservePendingUpload("Sam", "a.svg", 10, 100, true) +testing.assert(reserved.ok == true and pendingUploadEvents("sam").len == 1 and lockIsFree("upload-quota-sam"), "an upload is reserved and the quota lock released") +testing.assert(reservePendingUpload("sam", "b.svg", 95, 100, true).ok == false and lockIsFree("upload-quota-sam"), "a refused reservation releases the quota lock") +failing = true +testing.assert(try(reservePendingUpload("sam", "c.svg", 1, 100, true)).isErr(), "a failure while reserving reaches the caller") +testing.assert(lockIsFree("upload-quota-sam"), "the quota lock is released when reserving throws") +def releaseThrows() boolean ( + releasePendingUploads("sam", ["a.svg"]) + return true +) +testing.assert(try(releaseThrows()).isErr() and lockIsFree("upload-quota-sam"), "the quota lock is released when releasing throws") +failing = false +releasePendingUploads("sam", ["a.svg"]) +testing.assert(pendingUploadEvents("sam").len == 0 and lockIsFree("upload-quota-sam"), "the quota can be used again after a failure") diff --git a/src/projects/project-remix.osl b/src/projects/project-remix.osl index fe4e0f9..e27c85e 100644 --- a/src/projects/project-remix.osl +++ b/src/projects/project-remix.osl @@ -119,10 +119,9 @@ def handleGetRemixTree(*serve.Context c) ( ) def handleRemixProject(*serve.Context c) ( - string lockName = "project-history-" ++ c.param("id") - sync.lock(lockName) - handleRemixProjectLocked(c) - sync.unlock(lockName) + runLocked(projectSaveLock(c.param("id")), def() -> ( + handleRemixProjectLocked(c) + )) ) def handleRemixProjectLocked(*serve.Context c) ( diff --git a/src/projects/project-snapshots.osl b/src/projects/project-snapshots.osl index dfa3ccf..1a7902a 100644 --- a/src/projects/project-snapshots.osl +++ b/src/projects/project-snapshots.osl @@ -78,13 +78,13 @@ def removeProjectStorageKeyIfOrphaned(string key, string excludedId, boolean wor def cleanupReplacedProjectStorage(string id, string jsonKey, array workspaceKeys) ( string lockName = "project-history-" ++ id sync.lock(lockName) + defer sync.unlock(lockName) if jsonKey != "" ( removeProjectStorageKeyIfOrphaned(jsonKey, "", false) ) for i workspaceKeys.len ( removeProjectStorageKeyIfOrphaned(workspaceKeys[i].toStr(), "", true) ) - sync.unlock(lockName) ) def ensureProjectJsonSnapshot(object project) object ( @@ -140,11 +140,11 @@ def ensureProjectWorkspaceSnapshots(object project) object ( def maybeFlushProject(object project) ( string lockName = "project-history-" ++ project.id.toStr() sync.lock(lockName) + defer sync.unlock(lockName) auto latest = loadProject(project.id.toStr()) if latest.ok ( flushProjectLocked(latest.project.assert(object)) ) - sync.unlock(lockName) ) def flushProjectLocked(object project) ( diff --git a/src/projects/upload-parts.osl b/src/projects/upload-parts.osl index 6268440..7514474 100644 --- a/src/projects/upload-parts.osl +++ b/src/projects/upload-parts.osl @@ -104,34 +104,36 @@ def handleUploadProjectAsset(*serve.Context c) ( return ) + if storeProjectAssetLocked(c, project, name, file) ( + c.json(201, {ok: true, uploaded: true, name: name}) + ) +) + +def storeProjectAssetLocked(*serve.Context c, object project, string name, object file) boolean ( string assetLock = "asset-upload-" ++ name sync.lock(assetLock) + defer sync.unlock(assetLock) object assetsIndex = loadAssetsIndex() object knownAssets = assetsIndex.assets if knownAssets.contains(name) ( - sync.unlock(assetLock) c.json(200, {ok: true, uploaded: false, name: name}) - return + return false ) if !holdUploadAhead(c, project, name, file.size.toNum()) ( - sync.unlock(assetLock) - return + return false ) if !stageR2Blob(assetObjectKey(name), file.data, contentTypeForExt(extPartOf(name)), "") ( releaseUploadAhead(c, project, [name]) - sync.unlock(assetLock) c.internalError("asset upload failed") - return + return false ) knownAssets[name] = file.size if !saveAssetsIndex(assetsIndex) ( r2Remove(assetObjectKey(name)) - sync.unlock(assetLock) c.internalError("asset index update failed") - return + return false ) - sync.unlock(assetLock) - c.json(201, {ok: true, uploaded: true, name: name}) + return true ) def handleStageProjectUploadPart(*serve.Context c) ( @@ -352,19 +354,7 @@ def handleCompleteProjectUploadPart(*serve.Context c) ( c.badRequest("asset filename does not match its content") return ) - string assetLock = "asset-upload-" ++ name - sync.lock(assetLock) - object assetsIndex = loadAssetsIndex() - object knownAssets = assetsIndex.assets - boolean stored = knownAssets.contains(name) - if !stored ( - stored = stageR2Blob(assetObjectKey(name), data, contentTypeForExt(extPartOf(name)), "") - if stored ( - knownAssets[name] = metadata.size - stored = saveAssetsIndex(assetsIndex) - ) - ) - sync.unlock(assetLock) + boolean stored = storeStagedAssetLocked(name, data, metadata.size.toNum()) removeStagedUploadPart(token, metadata) if !stored ( c.internalError("could not store asset") @@ -390,3 +380,20 @@ def handleCompleteProjectUploadPart(*serve.Context c) ( ) c.json(201, {ok: true, complete: true, token: token, kind: kind}) ) + +def storeStagedAssetLocked(string name, byte[] data, number size) boolean ( + string assetLock = "asset-upload-" ++ name + sync.lock(assetLock) + defer sync.unlock(assetLock) + object assetsIndex = loadAssetsIndex() + object knownAssets = assetsIndex.assets + boolean stored = knownAssets.contains(name) + if !stored ( + stored = stageR2Blob(assetObjectKey(name), data, contentTypeForExt(extPartOf(name)), "") + if stored ( + knownAssets[name] = size + stored = saveAssetsIndex(assetsIndex) + ) + ) + return stored +) diff --git a/src/storage/backpack.osl b/src/storage/backpack.osl index bbd17a1..a8ac47a 100644 --- a/src/storage/backpack.osl +++ b/src/storage/backpack.osl @@ -118,44 +118,50 @@ def handleCreateBackpackItem(*serve.Context c) ( return ) ) - number itemBytes = bodyBytes + thumbnailBytes + object stored = storeBackpackItemLocked(c, username, userId, itemType, mime, bodyExt, body, bodyBytes, thumbnail, thumbnailExt, thumbnailBytes) + if stored.len == 0 ( + return + ) + object item = stored.item.assert(object) + if usesRoturStorage(userId) ( + thread.new(storageSettle, userId, stored.hold) + ) + c.json(200, {ok: true, item: backpackItemView(item)}) +) + +def storeBackpackItemLocked(*serve.Context c, string username, string userId, string itemType, string mime, string bodyExt, object body, number bodyBytes, object thumbnail, string thumbnailExt, number thumbnailBytes) object ( + number itemBytes = bodyBytes + thumbnailBytes string quotaLock = "upload-quota-" ++ normalizeUsername(username) sync.lock(quotaLock) + defer sync.unlock(quotaLock) sync.lock(backpackLockName(userId)) + defer sync.unlock(backpackLockName(userId)) array items = loadBackpackItems(userId) if items.len >= maxBackpackItems ( - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) c.json(413, {ok: false, code: "backpack_full", error: "Your backpack is full. Delete items you no longer need to make room."}) - return + return {} ) string folder = "backpack/" ++ randomString(32) object hold = noStorageHold() if usesRoturStorage(userId) ( hold = storageHold(userId, {backpack: [{bodyKey: folder, bodyBytes: bodyBytes}]}, !contextBool(c, "isAdmin")) if hold.ok != true ( - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) roturStorageFullResponse(c, hold.refusal.assertElse(object, {})) - return + return {} ) ) else ( object details = storageDetails(username) if !contextBool(c, "isAdmin") and details.used.toNum() + details.pending.toNum() + itemBytes > details.limit.toNum() ( - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) storageFullResponse(c, details, itemBytes) - return + return {} ) ) string bodyKey = folder ++ "/body." ++ bodyExt if !r2Put(bodyKey, body.data, contentTypeForExt(bodyExt)) ( storageHoldRelease(userId, hold) - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) c.internalError("failed to store backpack item") - return + return {} ) string thumbnailKey = "" if thumbnailExt != "" ( @@ -163,10 +169,8 @@ def handleCreateBackpackItem(*serve.Context c) ( if !r2Put(thumbnailKey, thumbnail.data, contentTypeForExt(thumbnailExt)) ( r2Remove(bodyKey) storageHoldRelease(userId, hold) - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) c.internalError("failed to store backpack thumbnail") - return + return {} ) ) object item = { @@ -186,17 +190,10 @@ def handleCreateBackpackItem(*serve.Context c) ( if !saveBackpackItems(userId, next) ( removeBackpackBlobs(item) storageHoldRelease(userId, hold) - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) c.internalError("failed to save backpack") - return + return {} ) - sync.unlock(backpackLockName(userId)) - sync.unlock(quotaLock) - if usesRoturStorage(userId) ( - thread.new(storageSettle, userId, hold) - ) - c.json(200, {ok: true, item: backpackItemView(item)}) + return {item: item, hold: hold} ) def handleUpdateBackpackItem(*serve.Context c) ( @@ -207,14 +204,28 @@ def handleUpdateBackpackItem(*serve.Context c) ( c.badRequest("nothing to update") return ) + object renamed = renameBackpackItemLocked(userId, id, body.name.toStr()) + if renamed.found != true ( + c.notFound("backpack item not found") + return + ) + if renamed.saved != true ( + c.internalError("failed to save backpack") + return + ) + c.json(200, {ok: true, item: backpackItemView(renamed.item.assert(object))}) +) + +def renameBackpackItemLocked(string userId, string id, string name) object ( sync.lock(backpackLockName(userId)) + defer sync.unlock(backpackLockName(userId)) array items = loadBackpackItems(userId) object updated = {} boolean found = false for i items.len ( object item = items[i].assert(object) if item.id.toStr() == id ( - item.name = backpackItemName(body.name.toStr(), item.type.toStr()) + item.name = backpackItemName(name, item.type.toStr()) item.updatedAt = timestamp items[i] = item updated = item @@ -222,32 +233,20 @@ def handleUpdateBackpackItem(*serve.Context c) ( ) ) if !found ( - sync.unlock(backpackLockName(userId)) - c.notFound("backpack item not found") - return - ) - boolean saved = saveBackpackItems(userId, items) - sync.unlock(backpackLockName(userId)) - if !saved ( - c.internalError("failed to save backpack") - return + return {found: false} ) - c.json(200, {ok: true, item: backpackItemView(updated)}) + return {found: true, saved: saveBackpackItems(userId, items), item: updated} ) def deleteBackpackItems(string userId, array ids) object ( - sync.lock(backpackLockName(userId)) - object split = backpackWithout(loadBackpackItems(userId), ids) + object split = removeBackpackItemsLocked(userId, ids) + if split.ok != true ( + return {ok: false} + ) array removed = split.removed.assert(array) if removed.len == 0 ( - sync.unlock(backpackLockName(userId)) return {ok: true, deleted: 0, freedBytes: 0} ) - if !saveBackpackItems(userId, split.kept.assert(array)) ( - sync.unlock(backpackLockName(userId)) - return {ok: false} - ) - sync.unlock(backpackLockName(userId)) for i removed.len ( removeBackpackBlobs(removed[i].assert(object)) ) @@ -255,6 +254,20 @@ def deleteBackpackItems(string userId, array ids) object ( return {ok: true, deleted: removed.len, freedBytes: backpackTotalBytes(removed)} ) +def removeBackpackItemsLocked(string userId, array ids) object ( + sync.lock(backpackLockName(userId)) + defer sync.unlock(backpackLockName(userId)) + object split = backpackWithout(loadBackpackItems(userId), ids) + array removed = split.removed.assert(array) + if removed.len == 0 ( + return {ok: true, removed: removed} + ) + if !saveBackpackItems(userId, split.kept.assert(array)) ( + return {ok: false} + ) + return {ok: true, removed: removed} +) + def handleDeleteBackpackItem(*serve.Context c) ( object res = deleteBackpackItems(contextString(c, "userId"), [c.param("id")]) if res.ok != true ( @@ -309,10 +322,10 @@ def collectBackpackForUser(string userId) array ( def purgeBackpackForUser(string userId) ( sync.lock(backpackLockName(userId)) + defer sync.unlock(backpackLockName(userId)) array items = loadBackpackItems(userId) for i items.len ( removeBackpackBlobs(items[i].assert(object)) ) void fs.remove(backpackIndexPath(userId)) - sync.unlock(backpackLockName(userId)) ) diff --git a/src/storage/backpack.test.osl b/src/storage/backpack.test.osl new file mode 100644 index 0000000..cfd2c85 --- /dev/null +++ b/src/storage/backpack.test.osl @@ -0,0 +1,154 @@ +import "std:testing" +import "std:fs" +import "std:json" +import "std:regex" +import "std:requests" +import "std:serve" +import "std:sync" +import "std:thread" +import "./backpack-policy.osl" + +string backpackDir = "/tmp/mistwarp-backpack-test/" +number maxBackpackItems = 10 +number maxBackpackBodyBytes = 1000 +number maxBackpackThumbnailBytes = 1000 + +boolean failing = false +object written = {} +def readFile(string path) object ( + if failing ( + throw "IOError: disk gone" + ) + if written.contains(path) ( + return {ok: true, data: written[path]} + ) + return {ok: false} +) +def writeFile(string path, object data) boolean ( + written[path] = data + return true +) +def normalizeUsername(string username) string ( + return username.toLower().strip() +) +def contextString(*serve.Context c, string key) string ( + if key == "username" ( + return "sam" + ) + if key == "userId" ( + return "u_sam" + ) + return "" +) +def contextBool(*serve.Context c, string key) boolean ( + return false +) +def usesRoturStorage(string userId) boolean ( + return false +) +def storageDetails(string username) object ( + return {used: 0, pending: 0, limit: 1000000} +) +def r2Put(string key, any body, string contentType) boolean ( + return true +) + +def contentTypeForExt(string ext) string ( + return "" +) +def noStorageHold() object ( + return {} +) +def paginate(array items, number offset, number limit) array ( + return [] +) +def publicUrl(string key) string ( + return "" +) +def r2Remove(string key) boolean ( + return false +) +def randomString(number length) string ( + return "" +) +def requestJSON(*serve.Context c) object ( + return {} +) +def roturStorageFullResponse(*serve.Context c, object refusal) ( + return +) +def storageFullResponse(*serve.Context c, object details, number neededBytes) ( + return +) +def storageHold(string userId, object changes, boolean enforce) object ( + return {} +) +def storageHoldRelease(string userId, object hold) ( + return +) +def storageSettleSoon(string userId) ( + return +) +def serveBlobKey(*serve.Context c, string key) ( + return +) +def storageSettle(string userId, object hold) ( + return +) +def lockIsFree(string lockName) boolean ( + boolean free = thread.new(def() -> ( + boolean taken = sync.tryLock(lockName) + if taken ( + sync.unlock(lockName) + ) + return taken + )).wait() + return free +) + +import "./backpack.osl" + +string backpackLock = backpackLockName("u_sam") +void saveBackpackItems("u_sam", [{id: "b1", type: "costume", name: "Cat", bodyKey: "backpack/x/body.svg", bodyBytes: 5, thumbnailKey: "", thumbnailBytes: 0}]) +testing.assert(renameBackpackItemLocked("u_sam", "b1", "Dog").saved == true and loadBackpackItems("u_sam")[1].name == "Dog" and lockIsFree(backpackLock), "an item is renamed and the backpack lock released") +failing = true +testing.assert(try(renameBackpackItemLocked("u_sam", "b1", "Fox")).isErr() and lockIsFree(backpackLock), "the backpack lock is released when renaming throws") +testing.assert(try(deleteBackpackItems("u_sam", ["b1"])).isErr() and lockIsFree(backpackLock), "the backpack lock is released when deleting throws") +def purgeThrows() boolean ( + purgeBackpackForUser("u_sam") + return true +) +testing.assert(try(purgeThrows()).isErr() and lockIsFree(backpackLock), "the backpack lock is released when purging throws") +failing = false +testing.assert(deleteBackpackItems("u_sam", ["b1"]).deleted == 1 and loadBackpackItems("u_sam").len == 0 and lockIsFree(backpackLock), "the backpack works again after a failure") + +*serve.Router app = serve.new() +app.POST("/backpack", handleCreateBackpackItem) +app.GET("/ready", def(*serve.Context c) -> ( + c.json(200, {ok: true}) +)) +def runApp() ( + app.run(":18354") +) +thread.new(runApp) +string base = "http://127.0.0.1:18354" +for i 50 ( + if requests.get(base ++ "/ready").isOk() ( + break + ) + wait 0.1 +) +string boundary = "smokeboundary" +def formPart(string name, string value) string ( + return "--" ++ boundary ++ "\r\nContent-Disposition: form-data; name=\"" ++ name ++ "\"\r\n\r\n" ++ value ++ "\r\n" +) +string form = formPart("type", "costume") ++ formPart("mime", "image/svg+xml") ++ formPart("name", "Cat") ++ "--" ++ boundary ++ "\r\nContent-Disposition: form-data; name=\"body\"; filename=\"body.svg\"\r\nContent-Type: image/svg+xml\r\n\r\n\r\n--" ++ boundary ++ "--\r\n" +object formOptions = {body: form, headers: {"Content-Type": "multipart/form-data; boundary=" ++ boundary}} + +failing = true +number failedStatus = requests.post(base ++ "/backpack", formOptions).unwrapOr({status: 0}).status.toNum() +testing.assert(failedStatus >= 500, "a failure while adding an item is a server error") +testing.assert(lockIsFree("upload-quota-sam") and lockIsFree(backpackLock), "the quota and backpack locks are both released when adding an item throws") +failing = false +number addedStatus = requests.post(base ++ "/backpack", formOptions).unwrapOr({status: 0}).status.toNum() +testing.assert(addedStatus == 200 and loadBackpackItems("u_sam").len == 1 and lockIsFree("upload-quota-sam") and lockIsFree(backpackLock), "items can be added again after a failure") diff --git a/src/storage/r2-upload-queue.osl b/src/storage/r2-upload-queue.osl index d93eaba..f650bd9 100644 --- a/src/storage/r2-upload-queue.osl +++ b/src/storage/r2-upload-queue.osl @@ -102,6 +102,7 @@ def stageR2Blob(string key, BlobBody body, string contentType, string contentEnc return false ) sync.lock("r2-upload-queue") + defer sync.unlock("r2-upload-queue") string path = localBlobPath(key) boolean saved = true if !fs.exists(path) ( @@ -111,7 +112,6 @@ def stageR2Blob(string key, BlobBody body, string contentType, string contentEnc if saved and !r2Local ( saved = queueR2UploadLocked(key, contentType, contentEncoding) ) - sync.unlock("r2-upload-queue") return saved ) @@ -120,15 +120,14 @@ def flushR2UploadQueue() ( return ) sync.lock("r2-upload-queue") + defer sync.unlock("r2-upload-queue") object activityResult = loadR2BlobActivity() if !activityResult.ok ( - sync.unlock("r2-upload-queue") return ) object activity = activityResult.activity.assert(object) object queueResult = readR2UploadQueue() if !queueResult.ok ( - sync.unlock("r2-upload-queue") return ) array items = queueResult.items.assert(array) @@ -180,7 +179,6 @@ def flushR2UploadQueue() ( ) ) writeFile(r2UploadQueueFile, {items: remaining}) - sync.unlock("r2-upload-queue") ) def removeQueuedR2UploadLocked(string key) boolean ( diff --git a/src/storage/r2.osl b/src/storage/r2.osl index 7e78a95..5d65e59 100644 --- a/src/storage/r2.osl +++ b/src/storage/r2.osl @@ -373,6 +373,7 @@ def r2ObjectType(string key) string ( def recordR2Stored(object item) ( sync.lock("r2-storage-index") + defer sync.unlock("r2-storage-index") auto loaded = readFile(r2StorageIndexFile) object data = if loaded.ok then loaded.data.assertElse(object, {}) else {} if !data.contains("objects") ( @@ -385,11 +386,11 @@ def recordR2Stored(object item) ( storedAt: timestamp } writeFile(r2StorageIndexFile, data) - sync.unlock("r2-storage-index") ) def recordR2Removed(string key) ( sync.lock("r2-storage-index") + defer sync.unlock("r2-storage-index") auto loaded = readFile(r2StorageIndexFile) if loaded.ok ( object data = loaded.data.assertElse(object, {}) @@ -400,7 +401,6 @@ def recordR2Removed(string key) ( writeFile(r2StorageIndexFile, data) ) ) - sync.unlock("r2-storage-index") ) def syncR2StorageInventory(boolean force) object ( @@ -408,10 +408,10 @@ def syncR2StorageInventory(boolean force) object ( return {ok: true, skipped: true, reason: "local_mode"} ) sync.lock("r2-storage-inventory-sync") + defer sync.unlock("r2-storage-inventory-sync") auto loaded = readFile(r2StorageIndexFile) object current = if loaded.ok then loaded.data.assertElse(object, {}) else {} if !force and current.contains("syncedAt") and current.syncedAt.toNum() > 0 ( - sync.unlock("r2-storage-inventory-sync") return {ok: true, skipped: true, syncedAt: current.syncedAt} ) number scanStartedAt = timestamp @@ -419,11 +419,19 @@ def syncR2StorageInventory(boolean force) object ( auto parsed = json.parse(encoded, {type: "object"}) if parsed.isErr() or parsed.unwrap().assert(object).ok != true ( string errorMessage = if parsed.isErr() then "could not read the R2 inventory" else parsed.unwrap().assert(object).error.toStr() - sync.unlock("r2-storage-inventory-sync") return {ok: false, error: errorMessage} ) object inventory = parsed.unwrap().assert(object) + boolean saved = saveR2InventoryLocked(inventory, scanStartedAt) + if !saved ( + return {ok: false, error: "could not save the R2 inventory"} + ) + return {ok: true, syncedAt: inventory.syncedAt, pages: inventory.pages} +) + +def saveR2InventoryLocked(object inventory, number scanStartedAt) boolean ( sync.lock("r2-storage-index") + defer sync.unlock("r2-storage-index") auto latestLoaded = readFile(r2StorageIndexFile) object latest = if latestLoaded.ok then latestLoaded.data.assertElse(object, {}) else {} object inventoryObjects = inventory.objects.assertElse(object, {}) @@ -436,17 +444,11 @@ def syncR2StorageInventory(boolean force) object ( inventoryObjects[key] = item ) ) - boolean saved = writeFile(r2StorageIndexFile, { + return writeFile(r2StorageIndexFile, { objects: inventoryObjects, syncedAt: inventory.syncedAt, pages: inventory.pages }) - sync.unlock("r2-storage-index") - sync.unlock("r2-storage-inventory-sync") - if !saved ( - return {ok: false, error: "could not save the R2 inventory"} - ) - return {ok: true, syncedAt: inventory.syncedAt, pages: inventory.pages} ) def r2Remove(string key) boolean ( @@ -454,6 +456,7 @@ def r2Remove(string key) boolean ( return false ) sync.lock("r2-upload-queue") + defer sync.unlock("r2-upload-queue") boolean removed = readR2UploadQueue().ok.assert(boolean) if removed and !r2Local ( auto deleted = r2Client.delete(key) @@ -469,7 +472,6 @@ def r2Remove(string key) boolean ( string path = localBlobPath(key) removed = !fs.exists(path) or fs.remove(path) ) - sync.unlock("r2-upload-queue") return removed ) diff --git a/src/storage/storage-ledger.osl b/src/storage/storage-ledger.osl index bf92121..4af3f15 100644 --- a/src/storage/storage-ledger.osl +++ b/src/storage/storage-ledger.osl @@ -473,9 +473,9 @@ def storageSettle(string userId, object hold) ( ) string lockName = "storage-ledger-" ++ userId sync.lock(lockName) + defer sync.unlock(lockName) object totals = ownerStorageTotals(userId, {}) settleTotalsLocked(userId, totals, hold) - sync.unlock(lockName) ) def settleTotalsLocked(string userId, object totals, object hold) ( @@ -523,23 +523,31 @@ def storageSettleSoon(string userId) ( if !usesRoturStorage(userId) or !ledgerReady() ( return ) + if !queueStorageSettleLocked(userId) ( + thread.new(settleQueued, userId) + ) +) + +def queueStorageSettleLocked(string userId) boolean ( sync.lock("storage-settle-queue") + defer sync.unlock("storage-settle-queue") boolean queued = storageSettleQueued.has(userId) if !queued ( storageSettleQueued.set(userId, true) ) - sync.unlock("storage-settle-queue") - if !queued ( - thread.new(settleQueued, userId) - ) + return queued ) def settleQueued(string userId) ( wait 2 + forgetStorageSettleLocked(userId) + storageSettle(userId, noStorageHold()) +) + +def forgetStorageSettleLocked(string userId) ( sync.lock("storage-settle-queue") + defer sync.unlock("storage-settle-queue") storageSettleQueued.delete(userId) - sync.unlock("storage-settle-queue") - storageSettle(userId, noStorageHold()) ) def storageSettleProject(object project) ( @@ -562,23 +570,25 @@ def settleEveryoneOnce() number ( number settled = 0 for i people.len ( string userId = people[i].toStr() - if usesRoturStorage(userId) ( - string lockName = "storage-ledger-" ++ userId - sync.lock(lockName) - // Totals from the sweep may be a moment old; work them out again for - // anyone whose count would change. - object listed = ledgerCounted(userId) - object mine = (totals[userId] ?? zero).assertElse(object, zero) - if listed.ok and ledgerChanges(listed.objects, mine).len > 0 ( - settleTotalsLocked(userId, ownerStorageTotals(userId, {}), noStorageHold()) - settled += 1 - ) - sync.unlock(lockName) + if usesRoturStorage(userId) and settleChangedLocked(userId, (totals[userId] ?? zero).assertElse(object, zero)) ( + settled += 1 ) ) return settled ) +def settleChangedLocked(string userId, object mine) boolean ( + string lockName = "storage-ledger-" ++ userId + sync.lock(lockName) + defer sync.unlock(lockName) + object listed = ledgerCounted(userId) + if listed.ok and ledgerChanges(listed.objects, mine).len > 0 ( + settleTotalsLocked(userId, ownerStorageTotals(userId, {}), noStorageHold()) + return true + ) + return false +) + def storageSettleLoop() ( wait 120 while true ( @@ -602,23 +612,35 @@ def pendingAssetHold(string ownerId, string username, string name, number bytes, return {ok: false, refusal: held} ) if held.ok ( - string lockName = "upload-quota-" ++ normalizeUsername(username) - sync.lock(lockName) - array events = pendingUploadEvents(username) - events.append({t: timestamp, bytes: bytes, name: name}) - writeFile(quotaPath(username), {pending: events}) - sync.unlock(lockName) + recordPendingAssetLocked(username, name, bytes) ) return {ok: true} ) +def recordPendingAssetLocked(string username, string name, number bytes) ( + string lockName = "upload-quota-" ++ normalizeUsername(username) + sync.lock(lockName) + defer sync.unlock(lockName) + array events = pendingUploadEvents(username) + events.append({t: timestamp, bytes: bytes, name: name}) + writeFile(quotaPath(username), {pending: events}) +) + // Releases the holds of assets a save now counts, or that failed to upload. def releasePendingAssetHolds(string ownerId, string username, array names) ( if names.len == 0 ( return ) + array releasing = forgetPendingAssetsLocked(username, names) + for i releasing.len ( + ledgerRelease(ownerId, "pending-" ++ releasing[i].toStr()) + ) +) + +def forgetPendingAssetsLocked(string username, array names) array ( string lockName = "upload-quota-" ++ normalizeUsername(username) sync.lock(lockName) + defer sync.unlock(lockName) array events = pendingUploadEvents(username) object wanted = {} for i names.len ( @@ -634,10 +656,7 @@ def releasePendingAssetHolds(string ownerId, string username, array names) ( if releasing.len > 0 ( writeFile(quotaPath(username), {pending: quotaWithoutNames(events, releasing, timestamp, quotaWindowMs)}) ) - sync.unlock(lockName) - for i releasing.len ( - ledgerRelease(ownerId, "pending-" ++ releasing[i].toStr()) - ) + return releasing ) // ---------- Backfill ---------- diff --git a/src/storage/storage-usage.osl b/src/storage/storage-usage.osl index 531b25c..117d646 100644 --- a/src/storage/storage-usage.osl +++ b/src/storage/storage-usage.osl @@ -30,19 +30,17 @@ def measuredHistoryBytes(object project, object inventory) number ( def backfillLiveProjectHistoryBytes(string id, object inventory) boolean ( string lockName = "project-history-" ++ id sync.lock(lockName) + defer sync.unlock(lockName) auto loaded = loadProject(id) if !loaded.ok ( - sync.unlock(lockName) return true ) object project = loaded.project.assert(object) if project.contains("historyBytes") ( - sync.unlock(lockName) return true ) number bytes = measuredHistoryBytes(project, inventory) if bytes < 0 ( - sync.unlock(lockName) return false ) project.historyBytes = bytes @@ -50,7 +48,6 @@ def backfillLiveProjectHistoryBytes(string id, object inventory) boolean ( if saved ( upsertIndexEntry(project) ) - sync.unlock(lockName) return saved ) diff --git a/src/storage/storage.osl b/src/storage/storage.osl index db1e85e..33c1d1e 100644 --- a/src/storage/storage.osl +++ b/src/storage/storage.osl @@ -274,8 +274,8 @@ def saveProject(object project) boolean ( ) // Keep the inactivity scan and eviction consistent with published edits. sync.lock("r2-upload-queue") + defer sync.unlock("r2-upload-queue") boolean saved = writeFile(projectsDir ++ id ++ ".json", project) - sync.unlock("r2-upload-queue") return saved ) From c699fc1bcf30c44644e8af9b2cc7ecea9005d2c3 Mon Sep 17 00:00:00 2001 From: Mistium <92952823+Mistium@users.noreply.github.com> Date: Sun, 1 Nov 2026 09:05:38 +0000 Subject: [PATCH 4/7] Cover the converted lock paths in the smoke test Adds pull comments, commit co-author credits, branch renames and deletes, reactions, staged assets, the backpack and the admin extension policies, with random comment and backpack ids normalized. --- scripts/e2e_smoke.py | 86 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 4 deletions(-) diff --git a/scripts/e2e_smoke.py b/scripts/e2e_smoke.py index 6031f02..09a14d4 100644 --- a/scripts/e2e_smoke.py +++ b/scripts/e2e_smoke.py @@ -29,7 +29,7 @@ "monitors": [], "extensions": [], "meta": {"semver": "3.0.0", "vm": "0.2.0", "agent": "smoke"}, } -USERS = {"sam": "u_sam", "alice": "u_alice", "bob": "u_bob", "dee": "u_dee"} +USERS = {"sam": "u_sam", "alice": "u_alice", "bob": "u_bob", "dee": "u_dee", "mist": "u_mist"} FULL_STORAGE = {"u_dee"} GONE_USERS = {"gone_user"} BUSY_USERS = {"busy_user"} @@ -40,7 +40,7 @@ SCOPED_SESSIONS = {"sam_editor": ("sam", "editor")} VOLATILE_NAMES = {"token", "viewKey", "expires", "created", "edited", "firstSeen", "historyBytes"} UNORDERED = {"largest"} -VOLATILE_FIELDS = {("notifications", "id"), ("largest", "bytes"), ("", "mistwarp")} +VOLATILE_FIELDS = {("notifications", "id"), ("largest", "bytes"), ("", "mistwarp"), ("comment", "id"), ("item", "id"), ("items", "id"), ("platform_data", "commentId")} SAVE_PATH = re.compile(r"^/v1/projects/([^/]+)(/upload)?$") HEX = re.compile(r"^[0-9a-f]{32,64}$") ISO = re.compile(r"^\d{4}-\d{2}-\d{2}T") @@ -229,6 +229,13 @@ def profile(self, username): with open(path) as handle: return json.load(handle) + def extension_policy(self): + path = os.path.join(self.dir, "data", "extensions.json") + if not os.path.exists(path): + return {} + with open(path) as handle: + return json.load(handle) + def read_project(self, project_id): with open(os.path.join(self.dir, "data", "projects", project_id + ".json")) as handle: return json.load(handle) @@ -264,6 +271,7 @@ def blob_inventory(self): EMBEDDED_HEX = re.compile(r"[0-9a-f]{32,64}") CONTENT_NAME = re.compile(r"^(assets/)?[0-9a-f]{32}\.[a-z0-9]+$|^project-json/[0-9a-f]{64}\.json\.gz$") ACCESS_KEY = re.compile(r"([?&]k=)[A-Za-z0-9]+") +BACKPACK_FOLDER = re.compile(r"(/blobs/backpack/)[A-Za-z0-9]{32}/") EMBEDDED_TIME = re.compile(r"(?" + path = "/v1/projects/" + shared = "/v1/projects/" + step("react invalid", "POST", shared + "/react", "alice", {"type": "wow"}) + step("react heart", "POST", shared + "/react", "alice", {"type": "heart"}) + step("react broken heart", "POST", shared + "/react", "alice", {"type": "brokenheart"}) + step("react missing project", "POST", "/v1/projects/nope/react", "alice", {"type": "heart"}) + step("react on a private project", "POST", path + "/react", "alice", {"type": "heart"}) + asset_name, asset_content = svg_asset("staged asset") + for label, name, content in (("staged asset", asset_name, asset_content), ("mismatched asset", svg_asset("never sent")[0], b"")): + status, started = step("stage " + label + " start", "POST", path + "/upload-parts", "sam", {"kind": "asset", "name": name, "size": len(content), "chunks": 1}) + token = started.get("token", "") if isinstance(started, dict) else "" + step("stage " + label + " chunk", "POST", path + "/upload-parts/%s/chunk" % token, "sam", form=({"index": "0"}, {"chunk": content})) + complete_part(step, path, "sam", token, "complete " + label) + step("check staged asset", "POST", path + "/assets/check", "sam", {"assets": [asset_name]}) + + for name, fields, files in ( + ("backpack unsupported type", {"type": "sound", "mime": "text/plain"}, {"body": b"x"}), + ("backpack missing body", {"type": "costume", "mime": "image/svg+xml"}, {}), + ("backpack add", {"type": "costume", "mime": "image/svg+xml", "name": "Cat"}, {"body": b""}), + ("backpack add with thumbnail", {"type": "costume", "mime": "image/svg+xml", "name": "Dog", "thumbnailMime": "image/png"}, {"body": b"", "thumbnail": b"\x89PNG backpack"}), + ): + step(name, "POST", "/v1/me/backpack", "sam", form=(fields, files)) + status, listed = step("backpack list", "GET", "/v1/me/backpack", "sam") + items = [item.get("id", "") for item in (listed.get("items") or [])] if isinstance(listed, dict) else [] + first = items[0] if items else "none" + step("backpack rename", "PATCH", "/v1/me/backpack/" + first, "sam", {"name": "Renamed"}) + step("backpack rename missing", "PATCH", "/v1/me/backpack/nope", "sam", {"name": "Renamed"}) + step("backpack rename without name", "PATCH", "/v1/me/backpack/" + first, "sam", {}) + step("backpack delete", "DELETE", "/v1/me/backpack/" + first, "sam") + step("backpack delete missing", "DELETE", "/v1/me/backpack/" + first, "sam") + step("backpack bulk delete", "POST", "/v1/me/backpack/delete", "sam", {"ids": items[1:] + ["nope"]}) + step("backpack bulk delete empty", "POST", "/v1/me/backpack/delete", "sam", {"ids": []}) + step("backpack after deletes", "GET", "/v1/me/backpack", "sam") + step("quota after backpack", "GET", "/v1/me/quota", "sam") + + records = server.extension_records() + known_hash = sorted(records)[0][:-len(".json")] if records else "0" * 64 + step("admin extension policy refused", "POST", "/v1/admin/extensions/policy", "sam", {"hash": known_hash, "status": "trusted"}) + step("admin extension policy invalid", "POST", "/v1/admin/extensions/policy", "mist", {"hash": "nope", "status": "trusted"}) + step("admin extension policy unknown", "POST", "/v1/admin/extensions/policy", "mist", {"hash": "f" * 64, "status": "trusted"}) + step("admin extension policy trusted", "POST", "/v1/admin/extensions/policy", "mist", {"hash": known_hash, "status": "trusted"}) + step("admin extension url blocked", "POST", "/v1/admin/extensions/url-policy", "mist", {"url": "https://example.com/blocked.js", "blocked": True}) + step("admin extension url blocked again", "POST", "/v1/admin/extensions/url-policy", "mist", {"url": "https://example.com/blocked.js", "blocked": True}) + step("admin extension url bad", "POST", "/v1/admin/extensions/url-policy", "mist", {"url": "ftp://example.com/x.js", "blocked": True}) + steps.append({"step": "extension policy after blocking", "status": 0, "body": {"blockedUrls": server.extension_policy().get("blockedUrls")}}) + step("admin extension url unblocked", "POST", "/v1/admin/extensions/url-policy", "mist", {"url": "https://example.com/blocked.js", "blocked": False}) + steps.append({"step": "extension policy after unblocking", "status": 0, "body": {"blockedUrls": server.extension_policy().get("blockedUrls")}}) + steps.append({"step": "extension records after policy changes", "status": 0, "body": server.extension_records()}) + + def volatile_key(key): return key in VOLATILE_NAMES @@ -807,7 +884,7 @@ def upload(): def normalize(value, ids, key="", parent=""): if isinstance(value, dict): - return {k: normalize(v, ids, k, key) for k, v in sorted(value.items())} + return {("" if HEX.match(k) else k): normalize(v, ids, k, key) for k, v in sorted(value.items())} if isinstance(value, list): items = [normalize(v, ids, key, parent) for v in value] return sorted(items, key=json.dumps) if key in UNORDERED else items @@ -819,7 +896,7 @@ def normalize(value, ids, key="", parent=""): value = value.replace(real, alias) if volatile_key(key) or HEX.match(value) or ISO.match(value): return "" - value = ACCESS_KEY.sub(r"\1", value) + value = BACKPACK_FOLDER.sub(r"\1/", ACCESS_KEY.sub(r"\1", value)) if CONTENT_NAME.match(value): return value return EMBEDDED_TIME.sub("