From 2e93c961c481f2a830441a8af1c9449a34d8ae77 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 25 Aug 2026 16:39:20 -0700 Subject: [PATCH 1/5] feat: expose subscription group memberships Add an account-scoped read model for subscription channels and their complete group assignments so the frontend can render and edit memberships without issuing one filtered subscription request per group. Constraint: Keep the shared SubscriptionItem unchanged because feeds, backups, imports, RSS, and recommendations reuse it Rejected: Add groupIds to SubscriptionItem | would query and serialize group data in unrelated paths Rejected: Fetch each group projection from the frontend | creates N requests for N groups Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep groupIds account-scoped and include empty arrays for ungrouped subscriptions Tested: JDK 25 clean check, 1,132 tests, shadowJar, OpenAPI validation, and live HTTP QA Not-tested: Frontend integration is intentionally deferred --- openapi.yaml | 2 + openapi/components/subscriptions.yaml | 12 +++++ openapi/paths/subscriptions.yaml | 14 ++++++ .../models/SubscriptionGroupMembershipItem.kt | 12 +++++ .../server/routes/SubscriptionsRoutes.kt | 5 ++ .../server/services/SubscriptionsService.kt | 43 ++++++++++++++-- .../server/SubscriptionGroupsRoutesTest.kt | 50 +++++++++++++++++++ 7 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipItem.kt diff --git a/openapi.yaml b/openapi.yaml index 849c1c99..ba7e65b9 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -45,6 +45,7 @@ paths: /saved-playlists: { $ref: ./openapi/paths/saved-playlists.yaml#/SavedPlaylists } /saved-playlists/{id}: { $ref: ./openapi/paths/saved-playlists.yaml#/SavedPlaylist } /subscriptions: { $ref: ./openapi/paths/subscriptions.yaml#/Subscriptions } + /subscriptions/group-memberships: { $ref: ./openapi/paths/subscriptions.yaml#/SubscriptionGroupMemberships } /subscriptions/groups: { $ref: ./openapi/paths/subscriptions.yaml#/SubscriptionGroups } /subscriptions/groups/{groupId}: { $ref: ./openapi/paths/subscriptions.yaml#/SubscriptionGroup } /subscriptions/groups/{groupId}/channels: { $ref: ./openapi/paths/subscriptions.yaml#/SubscriptionGroupChannels } @@ -160,6 +161,7 @@ components: SavedPlaylistItem: { $ref: ./openapi/components/media.yaml#/SavedPlaylistItem } SavedPlaylistRequest: { $ref: ./openapi/components/media.yaml#/SavedPlaylistRequest } SubscriptionItem: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionItem } + SubscriptionGroupMembershipItem: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupMembershipItem } SubscriptionGroupItem: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupItem } SubscriptionGroupRequest: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupRequest } SubscriptionGroupMembershipRequest: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupMembershipRequest } diff --git a/openapi/components/subscriptions.yaml b/openapi/components/subscriptions.yaml index 5894cc5f..c811f8a3 100644 --- a/openapi/components/subscriptions.yaml +++ b/openapi/components/subscriptions.yaml @@ -6,6 +6,18 @@ SubscriptionItem: name: { type: string } avatarUrl: { type: string } subscribedAt: { type: integer, format: int64 } +SubscriptionGroupMembershipItem: + type: object + required: [channelUrl, name, avatarUrl, subscribedAt, groupIds] + properties: + channelUrl: { type: string, minLength: 1 } + name: { type: string } + avatarUrl: { type: string } + subscribedAt: { type: integer, format: int64 } + groupIds: + type: array + uniqueItems: true + items: { type: string, format: uuid } SubscriptionCreateRequest: type: object required: [channelUrl, name, avatarUrl] diff --git a/openapi/paths/subscriptions.yaml b/openapi/paths/subscriptions.yaml index 9f9b4863..ac2dfa16 100644 --- a/openapi/paths/subscriptions.yaml +++ b/openapi/paths/subscriptions.yaml @@ -52,6 +52,20 @@ Subscriptions: '400': { $ref: ../components/common.yaml#/JsonError } '401': { $ref: ../components/common.yaml#/JsonError } '404': { $ref: ../components/common.yaml#/JsonError } +SubscriptionGroupMemberships: + get: + tags: [user-data] + summary: List subscriptions with their group memberships + description: Returns every current subscription once, with all account-owned group IDs assigned to that channel. Ungrouped subscriptions have an empty groupIds array. + responses: + '200': + description: Account-scoped subscriptions and their complete group memberships. + content: + application/json: + schema: + type: array + items: { $ref: ../components/subscriptions.yaml#/SubscriptionGroupMembershipItem } + '401': { $ref: ../components/common.yaml#/JsonError } SubscriptionGroups: get: tags: [user-data] diff --git a/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipItem.kt b/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipItem.kt new file mode 100644 index 00000000..b4a6fa60 --- /dev/null +++ b/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipItem.kt @@ -0,0 +1,12 @@ +package dev.typetype.server.models + +import kotlinx.serialization.Serializable + +@Serializable +data class SubscriptionGroupMembershipItem( + val channelUrl: String, + val name: String, + val avatarUrl: String, + val subscribedAt: Long, + val groupIds: List, +) diff --git a/src/main/kotlin/dev/typetype/server/routes/SubscriptionsRoutes.kt b/src/main/kotlin/dev/typetype/server/routes/SubscriptionsRoutes.kt index becd5d9b..33aac871 100644 --- a/src/main/kotlin/dev/typetype/server/routes/SubscriptionsRoutes.kt +++ b/src/main/kotlin/dev/typetype/server/routes/SubscriptionsRoutes.kt @@ -27,6 +27,11 @@ fun Route.subscriptionsRoutes( warmupService: HomeRecommendationWarmup = NoopHomeRecommendationWarmup, groupsService: SubscriptionGroupsService = SubscriptionGroupsService(), ) { + get("/subscriptions/group-memberships") { + call.withJwtAuth(authService) { userId -> + call.respond(subscriptionsService.getAllWithGroupMemberships(userId)) + } + } get("/subscriptions") { call.withJwtAuth(authService) { userId -> val parsed = call.parseSubscriptionSelection() diff --git a/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt b/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt index 6e90ab8f..70da17df 100644 --- a/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt +++ b/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt @@ -2,7 +2,9 @@ package dev.typetype.server.services import dev.typetype.server.db.DatabaseFactory import dev.typetype.server.db.tables.SubscriptionGroupMembershipsTable +import dev.typetype.server.db.tables.SubscriptionGroupsTable import dev.typetype.server.db.tables.SubscriptionsTable +import dev.typetype.server.models.SubscriptionGroupMembershipItem import dev.typetype.server.models.SubscriptionItem import org.jetbrains.exposed.v1.core.ResultRow import org.jetbrains.exposed.v1.core.SortOrder @@ -19,14 +21,37 @@ class SubscriptionsService { selection: SubscriptionSelection = SubscriptionSelection.All, ): List = DatabaseFactory.query { val selectedUrls = selectedChannelUrls(userId, selection) - val items = SubscriptionsTable.selectAll() - .where { SubscriptionsTable.userId eq userId } - .orderBy(SubscriptionsTable.subscribedAt to SortOrder.DESC) - .map { it.toItem() } + subscriptionItems(userId) .filter { selection == SubscriptionSelection.All || it.channelUrl in selectedUrls } - SubscriptionAvatarRepairer.repair(userId = userId, items = items) } + suspend fun getAllWithGroupMemberships(userId: String): List = + DatabaseFactory.query { + val groupIdsByChannel = SubscriptionGroupMembershipsTable + .innerJoin(SubscriptionGroupsTable) + .selectAll() + .where { + (SubscriptionGroupMembershipsTable.userId eq userId) and + (SubscriptionGroupsTable.userId eq userId) + } + .groupBy( + keySelector = { + ChannelUrlCanonicalizer.canonicalize(it[SubscriptionGroupMembershipsTable.channelUrl]) + }, + valueTransform = { it[SubscriptionGroupMembershipsTable.groupId] }, + ) + .mapValues { (_, groupIds) -> groupIds.sorted() } + subscriptionItems(userId).map { item -> + SubscriptionGroupMembershipItem( + channelUrl = item.channelUrl, + name = item.name, + avatarUrl = item.avatarUrl, + subscribedAt = item.subscribedAt, + groupIds = groupIdsByChannel[item.channelUrl].orEmpty(), + ) + } + } + suspend fun getChannelUrls(userId: String, selection: SubscriptionSelection): Set = DatabaseFactory.query { selectedChannelUrls(userId, selection) } @@ -55,6 +80,14 @@ class SubscriptionsService { SubscriptionsTable.deleteWhere { SubscriptionsTable.channelUrl eq canonicalUrl and (SubscriptionsTable.userId eq userId) } > 0 } + private fun subscriptionItems(userId: String): List { + val items = SubscriptionsTable.selectAll() + .where { SubscriptionsTable.userId eq userId } + .orderBy(SubscriptionsTable.subscribedAt to SortOrder.DESC) + .map { it.toItem() } + return SubscriptionAvatarRepairer.repair(userId = userId, items = items) + } + private fun selectedChannelUrls(userId: String, selection: SubscriptionSelection): Set { val all = SubscriptionsTable.selectAll() .where { SubscriptionsTable.userId eq userId } diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt index 70dd04b3..55c0fa76 100644 --- a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt +++ b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt @@ -6,6 +6,7 @@ import dev.typetype.server.routes.subscriptionGroupsRoutes import dev.typetype.server.routes.subscriptionsRoutes import dev.typetype.server.services.AuthService import dev.typetype.server.services.SubscriptionGroupsService +import dev.typetype.server.services.SubscriptionGroupWriteResult import dev.typetype.server.services.SubscriptionsService import io.ktor.client.request.delete import io.ktor.client.request.get @@ -25,6 +26,9 @@ import io.ktor.server.routing.routing import io.ktor.server.testing.ApplicationTestBuilder import io.ktor.server.testing.testApplication import kotlinx.serialization.json.Json +import kotlinx.serialization.json.jsonArray +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeAll @@ -37,6 +41,8 @@ class SubscriptionGroupsRoutesTest { private val auth = AuthService.fixed(TEST_USER_ID) companion object { + private const val FOREIGN_USER_ID = "foreign-user" + @BeforeAll @JvmStatic fun initDb() = TestDatabase.setup() @@ -61,6 +67,11 @@ class SubscriptionGroupsRoutesTest { assertEquals(HttpStatusCode.Unauthorized, client.get("/subscriptions/groups").status) } + @Test + fun `group membership projection requires authentication`() = withApp { + assertEquals(HttpStatusCode.Unauthorized, client.get("/subscriptions/group-memberships").status) + } + @Test fun `groups can be created listed renamed and deleted`() = withApp { val create = client.post("/subscriptions/groups") { @@ -121,6 +132,45 @@ class SubscriptionGroupsRoutesTest { assertTrue(authorizedGet("/subscriptions") { parameter("ungrouped", true) }.bodyAsText().contains(channel("one"))) } + @Test + fun `group membership projection returns account scoped memberships with subscription data`() = withApp { + val sharedChannel = channel("shared") + subscriptions.add(TEST_USER_ID, SubscriptionItem(sharedChannel, "Shared", "avatar")) + subscriptions.add(TEST_USER_ID, SubscriptionItem(channel("ungrouped"), "Ungrouped", "")) + subscriptions.add(FOREIGN_USER_ID, SubscriptionItem(sharedChannel, "Foreign shared", "")) + + val ownGroups = listOf(createGroup("Own"), createGroup("Another")) + ownGroups.forEach { group -> + assertEquals(HttpStatusCode.NoContent, client.put("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("""{"channelUrl":"$sharedChannel"}""") + }.status) + } + val foreignGroup = requireNotNull( + (groups.create(FOREIGN_USER_ID, "Foreign") as? SubscriptionGroupWriteResult.Success)?.group, + ) + groups.addSubscription(FOREIGN_USER_ID, foreignGroup.id, sharedChannel) + + val response = authorizedGet("/subscriptions/group-memberships") + assertEquals(HttpStatusCode.OK, response.status) + val items = Json.parseToJsonElement(response.bodyAsText()).jsonArray.map { it.jsonObject } + assertEquals(2, items.size) + + val shared = items.single { it.getValue("channelUrl").jsonPrimitive.content == sharedChannel } + assertEquals("Shared", shared.getValue("name").jsonPrimitive.content) + assertEquals("avatar", shared.getValue("avatarUrl").jsonPrimitive.content) + assertTrue(shared.getValue("subscribedAt").jsonPrimitive.content.toLong() > 0) + assertEquals( + ownGroups.map { it.id }.sorted(), + shared.getValue("groupIds").jsonArray.map { it.jsonPrimitive.content }, + ) + + val ungrouped = items.single { + it.getValue("channelUrl").jsonPrimitive.content == channel("ungrouped") + } + assertEquals(emptyList(), ungrouped.getValue("groupIds").jsonArray.map { it.jsonPrimitive.content }) + } + @Test fun `invalid or inaccessible filters fail explicitly`() = withApp { assertEquals(HttpStatusCode.BadRequest, authorizedGet("/subscriptions") { From a3a1530a92075ca8ad377fc2fad1da6ed58800ef Mon Sep 17 00:00:00 2001 From: User Date: Tue, 25 Aug 2026 17:16:34 -0700 Subject: [PATCH 2/5] feat: support bulk subscription group membership updates Let clients add or remove many subscribed channels from one group in a single account-scoped transaction while preserving the shipped singular request forms. Constraint: The singular membership contract shipped in v1.6.0 and must remain compatible Rejected: Cross-group membership delta endpoint | bulk organization is naturally scoped to one group Rejected: Repeated delete query parameters | encoded channel URLs can exceed practical URL limits Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep batch writes bounded, atomic, idempotent, and protected by SubscriptionMutationLock Tested: JDK 25 clean check, 1,134 tests, coverage, shadowJar, OpenAPI validation, focused retry regression, and live HTTP QA Not-tested: Frontend and Android integration are intentionally deferred --- openapi.yaml | 1 + openapi/components/subscriptions.yaml | 9 ++++ openapi/paths/subscriptions.yaml | 21 ++++++-- .../SubscriptionGroupMembershipRequest.kt | 5 +- .../server/routes/SubscriptionGroupsRoutes.kt | 50 ++++++++++++++---- .../services/SubscriptionGroupsService.kt | 51 +++++++++++++++---- .../server/SubscriptionGroupsRoutesTest.kt | 43 ++++++++++++++++ 7 files changed, 156 insertions(+), 24 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index ba7e65b9..63aec2da 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -165,6 +165,7 @@ components: SubscriptionGroupItem: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupItem } SubscriptionGroupRequest: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupRequest } SubscriptionGroupMembershipRequest: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupMembershipRequest } + SubscriptionGroupMembershipBatchRequest: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionGroupMembershipBatchRequest } SubscriptionFeedResponse: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionFeedResponse } SubscriptionFeedPreparingResponse: { $ref: ./openapi/components/subscriptions.yaml#/SubscriptionFeedPreparingResponse } RssFeedRequest: { $ref: ./openapi/components/rss.yaml#/RssFeedRequest } diff --git a/openapi/components/subscriptions.yaml b/openapi/components/subscriptions.yaml index c811f8a3..20a7476c 100644 --- a/openapi/components/subscriptions.yaml +++ b/openapi/components/subscriptions.yaml @@ -44,6 +44,15 @@ SubscriptionGroupMembershipRequest: required: [channelUrl] properties: channelUrl: { type: string, minLength: 1 } +SubscriptionGroupMembershipBatchRequest: + type: object + required: [channelUrls] + properties: + channelUrls: + type: array + minItems: 1 + maxItems: 500 + items: { type: string, minLength: 1 } SubscriptionFeedResponse: type: object required: [videos, nextpage, generation, generatedAt, refreshing] diff --git a/openapi/paths/subscriptions.yaml b/openapi/paths/subscriptions.yaml index ac2dfa16..7783c19c 100644 --- a/openapi/paths/subscriptions.yaml +++ b/openapi/paths/subscriptions.yaml @@ -131,12 +131,16 @@ SubscriptionGroupChannels: schema: { type: string, format: uuid } put: tags: [user-data] - summary: Add a subscribed channel to a group + summary: Add subscribed channels to a group + description: Accepts the original singular channelUrl request or up to 500 channelUrls. Batch additions are atomic, canonicalized and deduplicated. requestBody: required: true content: application/json: - schema: { $ref: ../components/subscriptions.yaml#/SubscriptionGroupMembershipRequest } + schema: + oneOf: + - { $ref: ../components/subscriptions.yaml#/SubscriptionGroupMembershipRequest } + - { $ref: ../components/subscriptions.yaml#/SubscriptionGroupMembershipBatchRequest } responses: '204': { description: Membership exists. } '400': { $ref: ../components/common.yaml#/JsonError } @@ -144,12 +148,21 @@ SubscriptionGroupChannels: '404': { $ref: ../components/common.yaml#/JsonError } delete: tags: [user-data] - summary: Remove a subscribed channel from a group + summary: Remove subscribed channels from a group + description: The url query parameter preserves the original singular contract. A JSON request body can remove one channel or up to 500 channelUrls atomically; absent batch memberships are ignored. parameters: - name: url in: query - required: true + required: false schema: { type: string, minLength: 1 } + requestBody: + required: false + content: + application/json: + schema: + oneOf: + - { $ref: ../components/subscriptions.yaml#/SubscriptionGroupMembershipRequest } + - { $ref: ../components/subscriptions.yaml#/SubscriptionGroupMembershipBatchRequest } responses: '204': { description: Membership deleted. } '400': { $ref: ../components/common.yaml#/JsonError } diff --git a/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipRequest.kt b/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipRequest.kt index 9a2fcb5f..d6befc62 100644 --- a/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipRequest.kt +++ b/src/main/kotlin/dev/typetype/server/models/SubscriptionGroupMembershipRequest.kt @@ -3,4 +3,7 @@ package dev.typetype.server.models import kotlinx.serialization.Serializable @Serializable -data class SubscriptionGroupMembershipRequest(val channelUrl: String) +data class SubscriptionGroupMembershipRequest( + val channelUrl: String? = null, + val channelUrls: List? = null, +) diff --git a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt index c80acb3d..be9f93c5 100644 --- a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt +++ b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt @@ -45,21 +45,33 @@ fun Route.subscriptionGroupsRoutes(groupsService: SubscriptionGroupsService, aut put("/subscriptions/groups/{groupId}/channels") { call.withJwtAuth(authService) { userId -> val groupId = call.groupId() ?: return@withJwtAuth call.respondMissingGroupId() - val request = runCatching { call.receive() }.getOrElse { - return@withJwtAuth call.respond(HttpStatusCode.BadRequest, ErrorResponse("Invalid request body")) + when (val request = call.receiveMembershipChannels() ?: return@withJwtAuth) { + is MembershipChannels.Single -> call.respondMembership( + groupsService.addSubscription(userId, groupId, request.channelUrl), + ) + is MembershipChannels.Batch -> call.respondMembership( + groupsService.addSubscriptions(userId, groupId, request.channelUrls), + ) } - if (request.channelUrl.isBlank()) { - return@withJwtAuth call.respond(HttpStatusCode.BadRequest, ErrorResponse("channelUrl must not be blank")) - } - call.respondMembership(groupsService.addSubscription(userId, groupId, request.channelUrl)) } } delete("/subscriptions/groups/{groupId}/channels") { call.withJwtAuth(authService) { userId -> val groupId = call.groupId() ?: return@withJwtAuth call.respondMissingGroupId() - val channelUrl = call.request.queryParameters["url"]?.takeIf(String::isNotBlank) - ?: return@withJwtAuth call.respond(HttpStatusCode.BadRequest, ErrorResponse("Missing channelUrl")) - call.respondMembership(groupsService.removeSubscription(userId, groupId, channelUrl)) + val queryChannelUrl = call.request.queryParameters["url"]?.takeIf(String::isNotBlank) + if (queryChannelUrl != null) { + return@withJwtAuth call.respondMembership( + groupsService.removeSubscription(userId, groupId, queryChannelUrl), + ) + } + when (val request = call.receiveMembershipChannels() ?: return@withJwtAuth) { + is MembershipChannels.Single -> call.respondMembership( + groupsService.removeSubscription(userId, groupId, request.channelUrl), + ) + is MembershipChannels.Batch -> call.respondMembership( + groupsService.removeSubscriptions(userId, groupId, request.channelUrls), + ) + } } } } @@ -72,6 +84,26 @@ private suspend fun ApplicationCall.receiveGroupRequest(): SubscriptionGroupRequ null } +private sealed interface MembershipChannels { + data class Single(val channelUrl: String) : MembershipChannels + data class Batch(val channelUrls: List) : MembershipChannels +} + +private suspend fun ApplicationCall.receiveMembershipChannels(): MembershipChannels? { + val request = runCatching { receive() }.getOrNull() + val channelUrl = request?.channelUrl?.takeIf(String::isNotBlank) + val channelUrls = request?.channelUrls + val parsed = when { + channelUrl != null && channelUrls == null -> MembershipChannels.Single(channelUrl) + request?.channelUrl == null && channelUrls != null && + channelUrls.size in 1..SubscriptionGroupsService.MAX_MEMBERSHIP_CHANNELS && + channelUrls.all(String::isNotBlank) -> MembershipChannels.Batch(channelUrls) + else -> null + } + if (parsed == null) respond(HttpStatusCode.BadRequest, ErrorResponse("Invalid request body")) + return parsed +} + private suspend fun ApplicationCall.respondGroupWrite(result: SubscriptionGroupWriteResult, created: Boolean) { when (result) { is SubscriptionGroupWriteResult.Success -> if (created) respond(HttpStatusCode.Created, result.group) else { diff --git a/src/main/kotlin/dev/typetype/server/services/SubscriptionGroupsService.kt b/src/main/kotlin/dev/typetype/server/services/SubscriptionGroupsService.kt index 9529ad45..5c92769b 100644 --- a/src/main/kotlin/dev/typetype/server/services/SubscriptionGroupsService.kt +++ b/src/main/kotlin/dev/typetype/server/services/SubscriptionGroupsService.kt @@ -9,6 +9,8 @@ import org.jetbrains.exposed.v1.core.ResultRow import org.jetbrains.exposed.v1.core.SortOrder import org.jetbrains.exposed.v1.core.and import org.jetbrains.exposed.v1.core.eq +import org.jetbrains.exposed.v1.core.inList +import org.jetbrains.exposed.v1.jdbc.batchInsert import org.jetbrains.exposed.v1.jdbc.deleteWhere import org.jetbrains.exposed.v1.jdbc.insertIgnore import org.jetbrains.exposed.v1.jdbc.selectAll @@ -103,19 +105,31 @@ class SubscriptionGroupsService { userId: String, groupId: String, rawChannelUrl: String, + ): SubscriptionGroupMembershipResult = addSubscriptions(userId, groupId, listOf(rawChannelUrl)) + + suspend fun addSubscriptions( + userId: String, + groupId: String, + rawChannelUrls: List, ): SubscriptionGroupMembershipResult = DatabaseFactory.query { SubscriptionMutationLock.acquire(userId) if (!groupExists(userId, groupId)) return@query SubscriptionGroupMembershipResult.GroupNotFound - val channelUrl = ChannelUrlCanonicalizer.canonicalize(rawChannelUrl) - val subscriptionExists = SubscriptionsTable.selectAll().where { - (SubscriptionsTable.userId eq userId) and (SubscriptionsTable.channelUrl eq channelUrl) - }.any() - if (!subscriptionExists) return@query SubscriptionGroupMembershipResult.SubscriptionNotFound - SubscriptionGroupMembershipsTable.insertIgnore { - it[SubscriptionGroupMembershipsTable.groupId] = groupId - it[SubscriptionGroupMembershipsTable.userId] = userId - it[SubscriptionGroupMembershipsTable.channelUrl] = channelUrl - it[addedAt] = System.currentTimeMillis() + val channelUrls = rawChannelUrls.mapTo(linkedSetOf(), ChannelUrlCanonicalizer::canonicalize) + val subscribed = SubscriptionsTable.selectAll().where { + (SubscriptionsTable.userId eq userId) and (SubscriptionsTable.channelUrl inList channelUrls) + }.mapTo(hashSetOf()) { it[SubscriptionsTable.channelUrl] } + if (subscribed.size != channelUrls.size) return@query SubscriptionGroupMembershipResult.SubscriptionNotFound + val existing = SubscriptionGroupMembershipsTable.selectAll().where { + (SubscriptionGroupMembershipsTable.groupId eq groupId) and + (SubscriptionGroupMembershipsTable.userId eq userId) and + (SubscriptionGroupMembershipsTable.channelUrl inList channelUrls) + }.mapTo(hashSetOf()) { it[SubscriptionGroupMembershipsTable.channelUrl] } + val addedAt = System.currentTimeMillis() + SubscriptionGroupMembershipsTable.batchInsert(channelUrls - existing, shouldReturnGeneratedValues = false) { url -> + this[SubscriptionGroupMembershipsTable.groupId] = groupId + this[SubscriptionGroupMembershipsTable.userId] = userId + this[SubscriptionGroupMembershipsTable.channelUrl] = url + this[SubscriptionGroupMembershipsTable.addedAt] = addedAt } SubscriptionGroupMembershipResult.Success } @@ -138,6 +152,22 @@ class SubscriptionGroupsService { } } + suspend fun removeSubscriptions( + userId: String, + groupId: String, + rawChannelUrls: List, + ): SubscriptionGroupMembershipResult = DatabaseFactory.query { + SubscriptionMutationLock.acquire(userId) + if (!groupExists(userId, groupId)) return@query SubscriptionGroupMembershipResult.GroupNotFound + val channelUrls = rawChannelUrls.mapTo(hashSetOf(), ChannelUrlCanonicalizer::canonicalize) + SubscriptionGroupMembershipsTable.deleteWhere { + (SubscriptionGroupMembershipsTable.groupId eq groupId) and + (SubscriptionGroupMembershipsTable.userId eq userId) and + (SubscriptionGroupMembershipsTable.channelUrl inList channelUrls) + } + SubscriptionGroupMembershipResult.Success + } + suspend fun getChannelUrls(userId: String, groupId: String): List = DatabaseFactory.query { SubscriptionGroupMembershipsTable.selectAll().where { (SubscriptionGroupMembershipsTable.groupId eq groupId) and @@ -182,6 +212,7 @@ class SubscriptionGroupsService { companion object { const val MAX_GROUP_NAME_LENGTH = 100 + const val MAX_MEMBERSHIP_CHANNELS = 500 private const val UNIQUE_VIOLATION_SQL_STATE = "23505" } } diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt index 55c0fa76..46ebea82 100644 --- a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt +++ b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt @@ -132,6 +132,49 @@ class SubscriptionGroupsRoutesTest { assertTrue(authorizedGet("/subscriptions") { parameter("ungrouped", true) }.bodyAsText().contains(channel("one"))) } + @Test + fun `membership routes add and remove multiple channels atomically`() = withApp { + val first = channel("one") + val second = channel("two") + val neverAdded = channel("three") + listOf(first, second, neverAdded).forEach { url -> + subscriptions.add(TEST_USER_ID, SubscriptionItem(url, url.substringAfterLast('/'), "")) + } + val group = createGroup("Work") + + assertEquals(HttpStatusCode.NoContent, client.put("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("""{"channelUrls":["$first","$second","$first"]}""") + }.status) + assertEquals(HttpStatusCode.NoContent, client.put("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("""{"channelUrls":["$first","$second"]}""") + }.status) + val grouped = authorizedGet("/subscriptions") { parameter("groupId", group.id) }.bodyAsText() + assertTrue(grouped.contains(first)) + assertTrue(grouped.contains(second)) + assertTrue(!grouped.contains(neverAdded)) + + assertEquals(HttpStatusCode.NoContent, client.delete("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("""{"channelUrls":["$first","$second","$neverAdded"]}""") + }.status) + assertEquals("[]", authorizedGet("/subscriptions") { parameter("groupId", group.id) }.bodyAsText()) + } + + @Test + fun `bulk membership addition changes nothing when a subscription is missing`() = withApp { + val subscribed = channel("subscribed") + subscriptions.add(TEST_USER_ID, SubscriptionItem(subscribed, "Subscribed", "")) + val group = createGroup("Work") + + assertEquals(HttpStatusCode.NotFound, client.put("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("""{"channelUrls":["$subscribed","${channel("missing")}"]}""") + }.status) + assertEquals("[]", authorizedGet("/subscriptions") { parameter("groupId", group.id) }.bodyAsText()) + } + @Test fun `group membership projection returns account scoped memberships with subscription data`() = withApp { val sharedChannel = channel("shared") From 27a3ff3e831b5309560e9ac413fd87a3ef6693af Mon Sep 17 00:00:00 2001 From: User Date: Tue, 25 Aug 2026 18:24:22 -0700 Subject: [PATCH 3/5] fix: reject ambiguous subscription group deletion Return a client error when a DELETE supplies both the legacy URL query parameter and a JSON membership body so the server cannot silently apply only part of the requested mutation. Constraint: Preserve both shipped query-only deletion and the new body-only batch contract Rejected: Give the query parameter precedence | silently ignores a valid batch body Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the two DELETE input forms mutually exclusive at the HTTP boundary Tested: Red-green route regression, JDK 25 check with 1,135 tests, coverage, shadowJar, OpenAPI validation, and live HTTP QA Not-tested: Frontend integration remains intentionally deferred --- openapi/paths/subscriptions.yaml | 2 +- .../server/routes/SubscriptionGroupsRoutes.kt | 7 +++++++ .../server/SubscriptionGroupsRoutesTest.kt | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/openapi/paths/subscriptions.yaml b/openapi/paths/subscriptions.yaml index 7783c19c..a765c764 100644 --- a/openapi/paths/subscriptions.yaml +++ b/openapi/paths/subscriptions.yaml @@ -149,7 +149,7 @@ SubscriptionGroupChannels: delete: tags: [user-data] summary: Remove subscribed channels from a group - description: The url query parameter preserves the original singular contract. A JSON request body can remove one channel or up to 500 channelUrls atomically; absent batch memberships are ignored. + description: The url query parameter preserves the original singular contract. A JSON request body can remove one channel or up to 500 channelUrls atomically; absent batch memberships are ignored. Supplying both url and a request body returns 400. parameters: - name: url in: query diff --git a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt index be9f93c5..452a994d 100644 --- a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt +++ b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt @@ -10,6 +10,7 @@ import dev.typetype.server.services.SubscriptionGroupsService import io.ktor.http.HttpStatusCode import io.ktor.server.application.ApplicationCall import io.ktor.server.request.receive +import io.ktor.server.request.receiveText import io.ktor.server.response.respond import io.ktor.server.routing.Route import io.ktor.server.routing.delete @@ -60,6 +61,12 @@ fun Route.subscriptionGroupsRoutes(groupsService: SubscriptionGroupsService, aut val groupId = call.groupId() ?: return@withJwtAuth call.respondMissingGroupId() val queryChannelUrl = call.request.queryParameters["url"]?.takeIf(String::isNotBlank) if (queryChannelUrl != null) { + if (call.receiveText().isNotBlank()) { + return@withJwtAuth call.respond( + HttpStatusCode.BadRequest, + ErrorResponse("Specify either url or a request body, not both"), + ) + } return@withJwtAuth call.respondMembership( groupsService.removeSubscription(userId, groupId, queryChannelUrl), ) diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt index 46ebea82..8ba7517c 100644 --- a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt +++ b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsRoutesTest.kt @@ -162,6 +162,26 @@ class SubscriptionGroupsRoutesTest { assertEquals("[]", authorizedGet("/subscriptions") { parameter("groupId", group.id) }.bodyAsText()) } + @Test + fun `membership deletion rejects query and body together`() = withApp { + val first = channel("one") + val second = channel("two") + listOf(first, second).forEach { url -> + subscriptions.add(TEST_USER_ID, SubscriptionItem(url, url.substringAfterLast('/'), "")) + } + val group = createGroup("Work") + listOf(first, second).forEach { url -> groups.addSubscription(TEST_USER_ID, group.id, url) } + + assertEquals(HttpStatusCode.BadRequest, client.delete("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + parameter("url", first) + setBody("""{"channelUrls":["$second"]}""") + }.status) + val grouped = authorizedGet("/subscriptions") { parameter("groupId", group.id) }.bodyAsText() + assertTrue(grouped.contains(first)) + assertTrue(grouped.contains(second)) + } + @Test fun `bulk membership addition changes nothing when a subscription is missing`() = withApp { val subscribed = channel("subscribed") From ca23ed86d5f126bc49c7d260d8b9d3343f593c08 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 25 Aug 2026 20:34:13 -0700 Subject: [PATCH 4/5] fix: bound subscription group membership requests Reject oversized membership bodies before they can be fully buffered and validate each submitted channel URL at the API boundary. Treat any non-empty DELETE body as present so whitespace cannot bypass query/body exclusivity. Constraint: Preserve the existing singular and batch membership contracts Rejected: Rely on Content-Length alone | chunked requests can omit the header Confidence: high Scope-risk: narrow Directive: Keep request limits aligned with the OpenAPI membership schemas Tested: ./gradlew --no-daemon clean check shadowJar validateOpenApi Tested: Live HTTP checks for body limits, URL length, and DELETE ambiguity --- openapi/components/subscriptions.yaml | 4 +- openapi/paths/subscriptions.yaml | 8 +- .../SubscriptionGroupMembershipRequestBody.kt | 85 +++++++++++ .../server/routes/SubscriptionGroupsRoutes.kt | 35 ++--- ...nGroupMembershipRequestLimitsRoutesTest.kt | 132 ++++++++++++++++++ 5 files changed, 233 insertions(+), 31 deletions(-) create mode 100644 src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupMembershipRequestBody.kt create mode 100644 src/test/kotlin/dev/typetype/server/SubscriptionGroupMembershipRequestLimitsRoutesTest.kt diff --git a/openapi/components/subscriptions.yaml b/openapi/components/subscriptions.yaml index 20a7476c..d0af4c01 100644 --- a/openapi/components/subscriptions.yaml +++ b/openapi/components/subscriptions.yaml @@ -43,7 +43,7 @@ SubscriptionGroupMembershipRequest: type: object required: [channelUrl] properties: - channelUrl: { type: string, minLength: 1 } + channelUrl: { type: string, minLength: 1, maxLength: 2048 } SubscriptionGroupMembershipBatchRequest: type: object required: [channelUrls] @@ -52,7 +52,7 @@ SubscriptionGroupMembershipBatchRequest: type: array minItems: 1 maxItems: 500 - items: { type: string, minLength: 1 } + items: { type: string, minLength: 1, maxLength: 2048 } SubscriptionFeedResponse: type: object required: [videos, nextpage, generation, generatedAt, refreshing] diff --git a/openapi/paths/subscriptions.yaml b/openapi/paths/subscriptions.yaml index a765c764..0a6244ab 100644 --- a/openapi/paths/subscriptions.yaml +++ b/openapi/paths/subscriptions.yaml @@ -132,7 +132,7 @@ SubscriptionGroupChannels: put: tags: [user-data] summary: Add subscribed channels to a group - description: Accepts the original singular channelUrl request or up to 500 channelUrls. Batch additions are atomic, canonicalized and deduplicated. + description: Accepts the original singular channelUrl request or up to 500 channelUrls. Batch additions are atomic, canonicalized and deduplicated. Request bodies are limited to 1 MiB. requestBody: required: true content: @@ -145,16 +145,17 @@ SubscriptionGroupChannels: '204': { description: Membership exists. } '400': { $ref: ../components/common.yaml#/JsonError } '401': { $ref: ../components/common.yaml#/JsonError } + '413': { $ref: ../components/common.yaml#/JsonError } '404': { $ref: ../components/common.yaml#/JsonError } delete: tags: [user-data] summary: Remove subscribed channels from a group - description: The url query parameter preserves the original singular contract. A JSON request body can remove one channel or up to 500 channelUrls atomically; absent batch memberships are ignored. Supplying both url and a request body returns 400. + description: The url query parameter preserves the original singular contract. A JSON request body can remove one channel or up to 500 channelUrls atomically; absent batch memberships are ignored. Supplying both url and any non-empty request body returns 400. Request bodies are limited to 1 MiB. parameters: - name: url in: query required: false - schema: { type: string, minLength: 1 } + schema: { type: string, minLength: 1, maxLength: 2048 } requestBody: required: false content: @@ -167,6 +168,7 @@ SubscriptionGroupChannels: '204': { description: Membership deleted. } '400': { $ref: ../components/common.yaml#/JsonError } '401': { $ref: ../components/common.yaml#/JsonError } + '413': { $ref: ../components/common.yaml#/JsonError } '404': { $ref: ../components/common.yaml#/JsonError } SubscriptionFeed: get: diff --git a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupMembershipRequestBody.kt b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupMembershipRequestBody.kt new file mode 100644 index 00000000..022aeaa6 --- /dev/null +++ b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupMembershipRequestBody.kt @@ -0,0 +1,85 @@ +package dev.typetype.server.routes + +import dev.typetype.server.models.ErrorResponse +import dev.typetype.server.models.SubscriptionGroupMembershipRequest +import dev.typetype.server.services.SubscriptionGroupsService +import io.ktor.http.ContentType +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import io.ktor.server.application.ApplicationCall +import io.ktor.server.request.contentType +import io.ktor.server.request.receiveChannel +import io.ktor.server.response.respond +import io.ktor.utils.io.ByteReadChannel +import io.ktor.utils.io.jvm.javaio.toInputStream +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import java.io.ByteArrayOutputStream + +private const val MAX_MEMBERSHIP_REQUEST_BODY_BYTES = 1024 * 1024 +private const val MAX_MEMBERSHIP_CHANNEL_URL_LENGTH = 2048 +private val membershipRequestJson = Json { ignoreUnknownKeys = true } + +internal sealed interface MembershipChannels { + data class Single(val channelUrl: String) : MembershipChannels + data class Batch(val channelUrls: List) : MembershipChannels +} + +internal suspend fun ApplicationCall.receiveMembershipChannels(body: ByteArray): MembershipChannels? { + val request = if (request.contentType().match(ContentType.Application.Json)) { + try { + membershipRequestJson.decodeFromString(body.decodeToString()) + } catch (_: SerializationException) { + null + } + } else { + null + } + val channelUrl = request?.channelUrl?.takeIf(String::isValidMembershipChannelUrl) + val channelUrls = request?.channelUrls + val parsed = when { + channelUrl != null && channelUrls == null -> MembershipChannels.Single(channelUrl) + request?.channelUrl == null && channelUrls != null && + channelUrls.size in 1..SubscriptionGroupsService.MAX_MEMBERSHIP_CHANNELS && + channelUrls.all(String::isValidMembershipChannelUrl) -> MembershipChannels.Batch(channelUrls) + else -> null + } + if (parsed == null) respond(HttpStatusCode.BadRequest, ErrorResponse("Invalid request body")) + return parsed +} + +internal suspend fun ApplicationCall.receiveMembershipBody(): ByteArray? { + val contentLength = request.headers[HttpHeaders.ContentLength]?.toLongOrNull() + if (contentLength != null && contentLength > MAX_MEMBERSHIP_REQUEST_BODY_BYTES) { + respondMembershipBodyTooLarge() + return null + } + val body = receiveChannel().readUpTo(MAX_MEMBERSHIP_REQUEST_BODY_BYTES) + if (body == null) respondMembershipBodyTooLarge() + return body +} + +private suspend fun ByteReadChannel.readUpTo(maxBytes: Int): ByteArray? = withContext(Dispatchers.IO) { + toInputStream().use { input -> + ByteArrayOutputStream().use { output -> + val buffer = ByteArray(DEFAULT_BUFFER_SIZE) + while (true) { + val read = input.read(buffer) + if (read <= 0) break + if (output.size() + read > maxBytes) return@withContext null + output.write(buffer, 0, read) + } + output.toByteArray() + } + } +} + +internal fun String.isValidMembershipChannelUrl(): Boolean = + isNotBlank() && length <= MAX_MEMBERSHIP_CHANNEL_URL_LENGTH + +private suspend fun ApplicationCall.respondMembershipBodyTooLarge() = respond( + HttpStatusCode.PayloadTooLarge, + ErrorResponse("Request body exceeds 1 MiB", "request_body_too_large"), +) diff --git a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt index 452a994d..2109f4b7 100644 --- a/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt +++ b/src/main/kotlin/dev/typetype/server/routes/SubscriptionGroupsRoutes.kt @@ -1,7 +1,6 @@ package dev.typetype.server.routes import dev.typetype.server.models.ErrorResponse -import dev.typetype.server.models.SubscriptionGroupMembershipRequest import dev.typetype.server.models.SubscriptionGroupRequest import dev.typetype.server.services.AuthService import dev.typetype.server.services.SubscriptionGroupMembershipResult @@ -10,7 +9,6 @@ import dev.typetype.server.services.SubscriptionGroupsService import io.ktor.http.HttpStatusCode import io.ktor.server.application.ApplicationCall import io.ktor.server.request.receive -import io.ktor.server.request.receiveText import io.ktor.server.response.respond import io.ktor.server.routing.Route import io.ktor.server.routing.delete @@ -46,7 +44,8 @@ fun Route.subscriptionGroupsRoutes(groupsService: SubscriptionGroupsService, aut put("/subscriptions/groups/{groupId}/channels") { call.withJwtAuth(authService) { userId -> val groupId = call.groupId() ?: return@withJwtAuth call.respondMissingGroupId() - when (val request = call.receiveMembershipChannels() ?: return@withJwtAuth) { + val body = call.receiveMembershipBody() ?: return@withJwtAuth + when (val request = call.receiveMembershipChannels(body) ?: return@withJwtAuth) { is MembershipChannels.Single -> call.respondMembership( groupsService.addSubscription(userId, groupId, request.channelUrl), ) @@ -59,9 +58,13 @@ fun Route.subscriptionGroupsRoutes(groupsService: SubscriptionGroupsService, aut delete("/subscriptions/groups/{groupId}/channels") { call.withJwtAuth(authService) { userId -> val groupId = call.groupId() ?: return@withJwtAuth call.respondMissingGroupId() - val queryChannelUrl = call.request.queryParameters["url"]?.takeIf(String::isNotBlank) + val queryChannelUrl = call.request.queryParameters["url"] + if (queryChannelUrl != null && !queryChannelUrl.isValidMembershipChannelUrl()) { + return@withJwtAuth call.respond(HttpStatusCode.BadRequest, ErrorResponse("Invalid channel URL")) + } + val body = call.receiveMembershipBody() ?: return@withJwtAuth if (queryChannelUrl != null) { - if (call.receiveText().isNotBlank()) { + if (body.isNotEmpty()) { return@withJwtAuth call.respond( HttpStatusCode.BadRequest, ErrorResponse("Specify either url or a request body, not both"), @@ -71,7 +74,7 @@ fun Route.subscriptionGroupsRoutes(groupsService: SubscriptionGroupsService, aut groupsService.removeSubscription(userId, groupId, queryChannelUrl), ) } - when (val request = call.receiveMembershipChannels() ?: return@withJwtAuth) { + when (val request = call.receiveMembershipChannels(body) ?: return@withJwtAuth) { is MembershipChannels.Single -> call.respondMembership( groupsService.removeSubscription(userId, groupId, request.channelUrl), ) @@ -91,26 +94,6 @@ private suspend fun ApplicationCall.receiveGroupRequest(): SubscriptionGroupRequ null } -private sealed interface MembershipChannels { - data class Single(val channelUrl: String) : MembershipChannels - data class Batch(val channelUrls: List) : MembershipChannels -} - -private suspend fun ApplicationCall.receiveMembershipChannels(): MembershipChannels? { - val request = runCatching { receive() }.getOrNull() - val channelUrl = request?.channelUrl?.takeIf(String::isNotBlank) - val channelUrls = request?.channelUrls - val parsed = when { - channelUrl != null && channelUrls == null -> MembershipChannels.Single(channelUrl) - request?.channelUrl == null && channelUrls != null && - channelUrls.size in 1..SubscriptionGroupsService.MAX_MEMBERSHIP_CHANNELS && - channelUrls.all(String::isNotBlank) -> MembershipChannels.Batch(channelUrls) - else -> null - } - if (parsed == null) respond(HttpStatusCode.BadRequest, ErrorResponse("Invalid request body")) - return parsed -} - private suspend fun ApplicationCall.respondGroupWrite(result: SubscriptionGroupWriteResult, created: Boolean) { when (result) { is SubscriptionGroupWriteResult.Success -> if (created) respond(HttpStatusCode.Created, result.group) else { diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionGroupMembershipRequestLimitsRoutesTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionGroupMembershipRequestLimitsRoutesTest.kt new file mode 100644 index 00000000..1b599dec --- /dev/null +++ b/src/test/kotlin/dev/typetype/server/SubscriptionGroupMembershipRequestLimitsRoutesTest.kt @@ -0,0 +1,132 @@ +package dev.typetype.server + +import dev.typetype.server.models.SubscriptionItem +import dev.typetype.server.routes.subscriptionGroupsRoutes +import dev.typetype.server.services.AuthService +import dev.typetype.server.services.SubscriptionGroupsService +import dev.typetype.server.services.SubscriptionGroupWriteResult +import dev.typetype.server.services.SubscriptionsService +import io.ktor.client.request.delete +import io.ktor.client.request.header +import io.ktor.client.request.parameter +import io.ktor.client.request.put +import io.ktor.client.request.setBody +import io.ktor.http.ContentType +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import io.ktor.http.content.OutgoingContent +import io.ktor.serialization.kotlinx.json.json +import io.ktor.server.application.install +import io.ktor.server.plugins.contentnegotiation.ContentNegotiation +import io.ktor.server.routing.routing +import io.ktor.server.testing.ApplicationTestBuilder +import io.ktor.server.testing.testApplication +import io.ktor.utils.io.ByteWriteChannel +import io.ktor.utils.io.writeByteArray +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +class SubscriptionGroupMembershipRequestLimitsRoutesTest { + private val groups = SubscriptionGroupsService() + private val subscriptions = SubscriptionsService() + private val auth = AuthService.fixed(TEST_USER_ID) + + companion object { + private const val EXPECTED_BODY_LIMIT_BYTES = 1024 * 1024 + private const val EXPECTED_CHANNEL_URL_LIMIT = 2048 + + @BeforeAll + @JvmStatic + fun initDb() = TestDatabase.setup() + } + + @BeforeEach + fun clean() = TestDatabase.truncateAll() + + @Test + fun `membership mutation rejects declared body over one mebibyte`() = withApp { + val group = createGroup() + + val response = client.put("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("x".repeat(EXPECTED_BODY_LIMIT_BYTES + 1)) + } + + assertEquals(HttpStatusCode.PayloadTooLarge, response.status) + } + + @Test + fun `membership mutation rejects streamed body over one mebibyte`() = withApp { + val group = createGroup() + + val response = client.put("/subscriptions/groups/${group.id}/channels") { + authorize() + setBody(oversizedStreamingBody()) + } + + assertEquals(HttpStatusCode.PayloadTooLarge, response.status) + } + + @Test + fun `membership mutation rejects channel urls over 2048 characters`() = withApp { + val group = createGroup() + val channelUrl = "https://example.com/" + "a".repeat(EXPECTED_CHANNEL_URL_LIMIT) + + val response = client.put("/subscriptions/groups/${group.id}/channels") { + authorizeJson() + setBody("""{"channelUrl":"$channelUrl"}""") + } + + assertEquals(HttpStatusCode.BadRequest, response.status) + } + + @Test + fun `membership deletion rejects query and whitespace body together`() = withApp { + val channelUrl = "https://example.com/channel" + subscriptions.add(TEST_USER_ID, SubscriptionItem(channelUrl, "Channel", "")) + val group = createGroup() + groups.addSubscription(TEST_USER_ID, group.id, channelUrl) + + val response = client.delete("/subscriptions/groups/${group.id}/channels") { + authorize() + parameter("url", channelUrl) + header(HttpHeaders.ContentType, ContentType.Text.Plain.toString()) + setBody(" ") + } + + assertEquals(HttpStatusCode.BadRequest, response.status) + assertEquals(1, groups.getAll(TEST_USER_ID).single().channelCount) + } + + private fun withApp(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication { + application { + install(ContentNegotiation) { json() } + routing { subscriptionGroupsRoutes(groups, auth) } + } + block() + } + + private suspend fun createGroup() = requireNotNull( + (groups.create(TEST_USER_ID, "Work") as? SubscriptionGroupWriteResult.Success)?.group, + ) + + private fun oversizedStreamingBody() = object : OutgoingContent.WriteChannelContent() { + override val contentType = ContentType.Application.Json + + override suspend fun writeTo(channel: ByteWriteChannel) { + val chunk = ByteArray(64 * 1024) { 'x'.code.toByte() } + repeat(EXPECTED_BODY_LIMIT_BYTES / chunk.size + 1) { channel.writeByteArray(chunk) } + } + } + + private fun io.ktor.client.request.HttpRequestBuilder.authorize() { + header(HttpHeaders.Authorization, "Bearer test-jwt") + } + + private fun io.ktor.client.request.HttpRequestBuilder.authorizeJson() { + authorize() + header(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + } +} From bdfe561a7d8fb298a73b16ad6d12d7258458d7a8 Mon Sep 17 00:00:00 2001 From: User Date: Wed, 26 Aug 2026 12:41:44 -0700 Subject: [PATCH 5/5] fix: keep subscription membership reads consistent Repair avatars only after applying subscription selection so unrelated channels neither consume the repair budget nor receive database writes. Serialize membership projections with account subscription mutations so channel data and group assignments come from one coherent state. Constraint: Preserve avatar repair for unfiltered and membership projection responses Rejected: Repair all subscriptions before filtering | unrelated rows consume the repair limit and receive writes Rejected: Add a new snapshot transaction API | the existing per-user mutation lock already serializes group changes Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep membership projections and mutations on SubscriptionMutationLock Tested: JDK 25 clean check, 1,141 tests, shadowJar, OpenAPI validation, and live HTTP lock-contention QA --- .../server/services/SubscriptionsService.kt | 9 ++--- .../server/SubscriptionGroupsServiceTest.kt | 34 ++++++++++++++++++ .../SubscriptionsAvatarRepairServiceTest.kt | 36 +++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt b/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt index 70da17df..b0bb5dd9 100644 --- a/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt +++ b/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt @@ -21,12 +21,14 @@ class SubscriptionsService { selection: SubscriptionSelection = SubscriptionSelection.All, ): List = DatabaseFactory.query { val selectedUrls = selectedChannelUrls(userId, selection) - subscriptionItems(userId) + val selectedItems = subscriptionItems(userId) .filter { selection == SubscriptionSelection.All || it.channelUrl in selectedUrls } + SubscriptionAvatarRepairer.repair(userId = userId, items = selectedItems) } suspend fun getAllWithGroupMemberships(userId: String): List = DatabaseFactory.query { + SubscriptionMutationLock.acquire(userId) val groupIdsByChannel = SubscriptionGroupMembershipsTable .innerJoin(SubscriptionGroupsTable) .selectAll() @@ -41,7 +43,7 @@ class SubscriptionsService { valueTransform = { it[SubscriptionGroupMembershipsTable.groupId] }, ) .mapValues { (_, groupIds) -> groupIds.sorted() } - subscriptionItems(userId).map { item -> + SubscriptionAvatarRepairer.repair(userId = userId, items = subscriptionItems(userId)).map { item -> SubscriptionGroupMembershipItem( channelUrl = item.channelUrl, name = item.name, @@ -81,11 +83,10 @@ class SubscriptionsService { } private fun subscriptionItems(userId: String): List { - val items = SubscriptionsTable.selectAll() + return SubscriptionsTable.selectAll() .where { SubscriptionsTable.userId eq userId } .orderBy(SubscriptionsTable.subscribedAt to SortOrder.DESC) .map { it.toItem() } - return SubscriptionAvatarRepairer.repair(userId = userId, items = items) } private fun selectedChannelUrls(userId: String, selection: SubscriptionSelection): Set { diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsServiceTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsServiceTest.kt index 4233deae..7f017a97 100644 --- a/src/test/kotlin/dev/typetype/server/SubscriptionGroupsServiceTest.kt +++ b/src/test/kotlin/dev/typetype/server/SubscriptionGroupsServiceTest.kt @@ -212,6 +212,40 @@ class SubscriptionGroupsServiceTest { assertTrue(allWaited, "all group mutations must wait for the account-scoped lock") } + @Test + fun `membership projection shares the account subscription lock`() = runTest { + val userId = "projection-user" + val group = groups.create(userId, "Group").createdGroup() + subscriptions.add(userId, subscription("one")) + groups.addSubscription(userId, group.id, channel("one")) + val lockHeld = CountDownLatch(1) + val releaseLock = CountDownLatch(1) + val holder = async(Dispatchers.IO) { + DatabaseFactory.query { + TransactionManager.current().exec(subscriptionLockSql(userId)) + lockHeld.countDown() + check(releaseLock.await(5, TimeUnit.SECONDS)) + } + } + assertTrue(lockHeld.await(5, TimeUnit.SECONDS)) + + val projection = async(Dispatchers.IO) { subscriptions.getAllWithGroupMemberships(userId) } + val readWaited = try { + withContext(Dispatchers.IO) { + withTimeoutOrNull(2_000L) { + while (!projection.isCompleted && waitingSubscriptionLocks(userId) == 0) yield() + !projection.isCompleted + } ?: false + } + } finally { + releaseLock.countDown() + } + + holder.await() + assertTrue(readWaited, "the membership projection must wait for the account-scoped lock") + assertEquals(listOf(group.id), projection.await().single().groupIds) + } + @Test fun `replacement imports retain only memberships for subscriptions still present`() = runTest { val group = groups.create("user", "Group").createdGroup() diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionsAvatarRepairServiceTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionsAvatarRepairServiceTest.kt index d0f988c8..de4341ef 100644 --- a/src/test/kotlin/dev/typetype/server/SubscriptionsAvatarRepairServiceTest.kt +++ b/src/test/kotlin/dev/typetype/server/SubscriptionsAvatarRepairServiceTest.kt @@ -3,12 +3,19 @@ package dev.typetype.server import dev.typetype.server.db.DatabaseFactory import dev.typetype.server.db.tables.FavoritesTable import dev.typetype.server.db.tables.HistoryTable +import dev.typetype.server.db.tables.SubscriptionsTable import dev.typetype.server.db.tables.WatchLaterTable import dev.typetype.server.models.SubscriptionItem import dev.typetype.server.services.SubscriptionAvatarRepairer +import dev.typetype.server.services.SubscriptionGroupWriteResult +import dev.typetype.server.services.SubscriptionGroupsService +import dev.typetype.server.services.SubscriptionSelection import dev.typetype.server.services.SubscriptionsService import kotlinx.coroutines.test.runTest +import org.jetbrains.exposed.v1.core.and +import org.jetbrains.exposed.v1.core.eq import org.jetbrains.exposed.v1.jdbc.insert +import org.jetbrains.exposed.v1.jdbc.selectAll import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.BeforeEach @@ -16,6 +23,7 @@ import org.junit.jupiter.api.Test class SubscriptionsAvatarRepairServiceTest { private val service = SubscriptionsService() + private val groups = SubscriptionGroupsService() companion object { const val WATCH_CHANNEL_URL = "https://www.youtube.com/channel/UCWatch" @@ -62,6 +70,28 @@ class SubscriptionsAvatarRepairServiceTest { assertEquals(26, second.count { it.avatarUrl.isNotBlank() }) } + @Test + fun `filtered getAll repairs avatars only for selected subscriptions`() = runTest { + repeat(25) { index -> + val channelUrl = "https://www.youtube.com/channel/UCUnselected$index" + addSubscription(channelUrl) + addHistory( + channelUrl = channelUrl, + avatarUrl = "https://avatar.test/unselected-$index.jpg", + watchedAt = (index + 1).toLong(), + ) + } + addSubscription(WATCH_CHANNEL_URL) + addHistory(channelUrl = WATCH_CHANNEL_URL, avatarUrl = WATCH_AVATAR_URL, watchedAt = 0) + val group = (groups.create(TEST_USER_ID, "Selected") as SubscriptionGroupWriteResult.Success).group + groups.addSubscription(TEST_USER_ID, group.id, WATCH_CHANNEL_URL) + + val selected = service.getAll(TEST_USER_ID, SubscriptionSelection.Group(group.id)).single() + + assertEquals(WATCH_AVATAR_URL, selected.avatarUrl) + assertEquals("", storedAvatar("https://www.youtube.com/channel/UCUnselected0")) + } + @Test fun `avatar repair scans past unrepairable empty subscriptions`() = runTest { addWatchLater(channelUrl = WATCH_CHANNEL_URL, avatarUrl = WATCH_AVATAR_URL) @@ -78,6 +108,12 @@ class SubscriptionsAvatarRepairServiceTest { service.add(TEST_USER_ID, SubscriptionItem(channelUrl = channelUrl, name = "Channel", avatarUrl = "")) } + private suspend fun storedAvatar(channelUrl: String): String = DatabaseFactory.query { + SubscriptionsTable.selectAll().where { + (SubscriptionsTable.userId eq TEST_USER_ID) and (SubscriptionsTable.channelUrl eq channelUrl) + }.single()[SubscriptionsTable.avatarUrl] + } + private suspend fun addWatchLater(channelUrl: String, avatarUrl: String): Unit = DatabaseFactory.query { WatchLaterTable.insert { it[userId] = TEST_USER_ID; it[url] = "https://video.test/watch"; it[title] = "Video"; it[thumbnail] = ""