diff --git a/src/main/kotlin/dev/typetype/server/db/DatabaseFactory.kt b/src/main/kotlin/dev/typetype/server/db/DatabaseFactory.kt index b482cc0..0c29d31 100644 --- a/src/main/kotlin/dev/typetype/server/db/DatabaseFactory.kt +++ b/src/main/kotlin/dev/typetype/server/db/DatabaseFactory.kt @@ -110,6 +110,7 @@ object DatabaseFactory { exec("CREATE UNIQUE INDEX IF NOT EXISTS users_public_username_unique ON users (public_username)") DatabasePrimaryKeyMigrations.apply() DatabaseIndexMigrations.apply() + DatabaseSubscriptionsCanonicalMigration.apply() } } diff --git a/src/main/kotlin/dev/typetype/server/db/DatabaseSubscriptionsCanonicalMigration.kt b/src/main/kotlin/dev/typetype/server/db/DatabaseSubscriptionsCanonicalMigration.kt new file mode 100644 index 0000000..74bf1ec --- /dev/null +++ b/src/main/kotlin/dev/typetype/server/db/DatabaseSubscriptionsCanonicalMigration.kt @@ -0,0 +1,54 @@ +package dev.typetype.server.db + +import dev.typetype.server.db.tables.SubscriptionsTable +import dev.typetype.server.services.ChannelUrlCanonicalizer +import org.jetbrains.exposed.v1.jdbc.deleteAll +import org.jetbrains.exposed.v1.jdbc.insert +import org.jetbrains.exposed.v1.jdbc.selectAll + +object DatabaseSubscriptionsCanonicalMigration { + fun apply() { + val rows = SubscriptionsTable.selectAll().map { + SubscriptionRow( + userId = it[SubscriptionsTable.userId], + channelUrl = it[SubscriptionsTable.channelUrl], + name = it[SubscriptionsTable.name], + avatarUrl = it[SubscriptionsTable.avatarUrl], + subscribedAt = it[SubscriptionsTable.subscribedAt], + ) + } + if (rows.isEmpty()) return + val normalized = rows.map { row -> + row.copy(channelUrl = ChannelUrlCanonicalizer.canonicalize(row.channelUrl)) + } + val deduped = normalized + .groupBy { it.userId to it.channelUrl } + .values + .map { group -> + group.maxWith(compareBy { it.subscribedAt } + .thenBy { it.name } + .thenBy { it.avatarUrl }) + } + val needsCanonicalization = rows.zip(normalized).any { (before, after) -> before.channelUrl != after.channelUrl } + val needsDeduplication = deduped.size != normalized.size + if (!needsCanonicalization && !needsDeduplication) return + SubscriptionsTable.deleteAll() + deduped.forEach { row -> + SubscriptionsTable.insert { + it[SubscriptionsTable.userId] = row.userId + it[SubscriptionsTable.channelUrl] = row.channelUrl + it[SubscriptionsTable.name] = row.name + it[SubscriptionsTable.avatarUrl] = row.avatarUrl + it[SubscriptionsTable.subscribedAt] = row.subscribedAt + } + } + } + + private data class SubscriptionRow( + val userId: String, + val channelUrl: String, + val name: String, + val avatarUrl: String, + val subscribedAt: Long, + ) +} diff --git a/src/main/kotlin/dev/typetype/server/services/ChannelUrlCanonicalizer.kt b/src/main/kotlin/dev/typetype/server/services/ChannelUrlCanonicalizer.kt new file mode 100644 index 0000000..1ceb388 --- /dev/null +++ b/src/main/kotlin/dev/typetype/server/services/ChannelUrlCanonicalizer.kt @@ -0,0 +1,39 @@ +package dev.typetype.server.services + +import java.net.URI + +object ChannelUrlCanonicalizer { + fun canonicalize(raw: String): String { + val trimmed = raw.trim() + if (trimmed.isEmpty()) return trimmed + val uri = runCatching { URI(trimmed) }.getOrNull() ?: return trimmed + val host = uri.host?.lowercase()?.takeIf { it.isNotBlank() } ?: return trimmed + val path = normalizePath(uri.path) + val port = normalizePort(uri.port) + return runCatching { + URI( + "https", + uri.userInfo, + host, + port, + path, + null, + null, + ).toString() + }.getOrDefault(trimmed) + } + + private fun normalizePath(rawPath: String?): String { + if (rawPath.isNullOrBlank()) return "" + var path: String = rawPath + while (path.length > 1 && path.endsWith('/')) { + path = path.dropLast(1) + } + return if (path == "/") "" else path + } + + private fun normalizePort(rawPort: Int): Int { + if (rawPort == 80 || rawPort == 443) return -1 + return rawPort + } +} diff --git a/src/main/kotlin/dev/typetype/server/services/PipePipeBackupPersisterService.kt b/src/main/kotlin/dev/typetype/server/services/PipePipeBackupPersisterService.kt index 521dcdc..8638661 100644 --- a/src/main/kotlin/dev/typetype/server/services/PipePipeBackupPersisterService.kt +++ b/src/main/kotlin/dev/typetype/server/services/PipePipeBackupPersisterService.kt @@ -55,8 +55,9 @@ class PipePipeBackupPersisterService { private fun insertSubscriptions(userId: String, items: List): Int = items.sumOf { item -> + val canonicalUrl = ChannelUrlCanonicalizer.canonicalize(item.url) SubscriptionsTable.insertIgnore { - it[SubscriptionsTable.userId] = userId; it[channelUrl] = item.url; it[name] = item.name; it[avatarUrl] = item.avatarUrl; it[subscribedAt] = System.currentTimeMillis() + it[SubscriptionsTable.userId] = userId; it[channelUrl] = canonicalUrl; it[name] = item.name; it[avatarUrl] = item.avatarUrl; it[subscribedAt] = System.currentTimeMillis() }.insertedCount } diff --git a/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt b/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt index 71c0cd0..c74c84e 100644 --- a/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt +++ b/src/main/kotlin/dev/typetype/server/services/SubscriptionsService.kt @@ -21,25 +21,27 @@ class SubscriptionsService { } suspend fun add(userId: String, item: SubscriptionItem): SubscriptionItem { + val canonicalUrl = ChannelUrlCanonicalizer.canonicalize(item.channelUrl) val now = System.currentTimeMillis() DatabaseFactory.query { SubscriptionsTable.insert { it[SubscriptionsTable.userId] = userId - it[channelUrl] = item.channelUrl + it[channelUrl] = canonicalUrl it[name] = item.name it[avatarUrl] = item.avatarUrl it[subscribedAt] = now } } - return item.copy(subscribedAt = now) + return item.copy(channelUrl = canonicalUrl, subscribedAt = now) } suspend fun delete(userId: String, channelUrl: String): Boolean = DatabaseFactory.query { - SubscriptionsTable.deleteWhere { SubscriptionsTable.channelUrl eq channelUrl and (SubscriptionsTable.userId eq userId) } > 0 + val canonicalUrl = ChannelUrlCanonicalizer.canonicalize(channelUrl) + SubscriptionsTable.deleteWhere { SubscriptionsTable.channelUrl eq canonicalUrl and (SubscriptionsTable.userId eq userId) } > 0 } private fun ResultRow.toItem() = SubscriptionItem( - channelUrl = this[SubscriptionsTable.channelUrl], + channelUrl = ChannelUrlCanonicalizer.canonicalize(this[SubscriptionsTable.channelUrl]), name = this[SubscriptionsTable.name], avatarUrl = this[SubscriptionsTable.avatarUrl], subscribedAt = this[SubscriptionsTable.subscribedAt], diff --git a/src/main/kotlin/dev/typetype/server/services/YoutubeTakeoutImporterService.kt b/src/main/kotlin/dev/typetype/server/services/YoutubeTakeoutImporterService.kt index bf60d6c..b547ed0 100644 --- a/src/main/kotlin/dev/typetype/server/services/YoutubeTakeoutImporterService.kt +++ b/src/main/kotlin/dev/typetype/server/services/YoutubeTakeoutImporterService.kt @@ -22,7 +22,7 @@ class YoutubeTakeoutImporterService( val existingSubsDeferred = async { subscriptionsService.getAll(userId).map { it.channelUrl }.toSet() } val existingPlaylistsDeferred = async { playlistService.getAll(userId) } val sourceMappingsDeferred = async { playlistKeyService.getMappings(userId).toMutableMap() } - val existingSubs = existingSubsDeferred.await() + val existingSubs = existingSubsDeferred.await().toMutableSet() val existingPlaylistRows = existingPlaylistsDeferred.await() val existingPlaylists = existingPlaylistRows.associateBy { it.name.lowercase() } val sourceMappings = sourceMappingsDeferred.await() @@ -32,8 +32,10 @@ class YoutubeTakeoutImporterService( var subSkipped = 0 if (plan.importSubscriptions) { parsed.subscriptions.forEach { item -> - if (item.channelUrl in existingSubs) subSkipped += 1 else { + val canonicalUrl = ChannelUrlCanonicalizer.canonicalize(item.channelUrl) + if (canonicalUrl in existingSubs) subSkipped += 1 else { subscriptionsService.add(userId, SubscriptionItem(item.channelUrl, item.name, item.avatarUrl)) + existingSubs += canonicalUrl subImported += 1 } } diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionsCanonicalMigrationTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionsCanonicalMigrationTest.kt new file mode 100644 index 0000000..47b66cf --- /dev/null +++ b/src/test/kotlin/dev/typetype/server/SubscriptionsCanonicalMigrationTest.kt @@ -0,0 +1,65 @@ +package dev.typetype.server + +import dev.typetype.server.db.DatabaseSubscriptionsCanonicalMigration +import dev.typetype.server.db.tables.SubscriptionsTable +import org.jetbrains.exposed.v1.core.SortOrder +import org.jetbrains.exposed.v1.core.eq +import org.jetbrains.exposed.v1.jdbc.insert +import org.jetbrains.exposed.v1.jdbc.selectAll +import org.jetbrains.exposed.v1.jdbc.transactions.transaction +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 SubscriptionsCanonicalMigrationTest { + companion object { + @BeforeAll + @JvmStatic + fun initDb() = TestDatabase.setup() + } + + @BeforeEach + fun clean() { + TestDatabase.truncateAll() + } + + @Test + fun `migration canonicalizes and deduplicates subscriptions per user`() { + transaction { + SubscriptionsTable.insert { + it[userId] = TEST_USER_ID + it[channelUrl] = "https://www.youtube.com/channel/UC123" + it[name] = "old" + it[avatarUrl] = "a" + it[subscribedAt] = 10L + } + SubscriptionsTable.insert { + it[userId] = TEST_USER_ID + it[channelUrl] = "http://WWW.YouTube.com/channel/UC123/?x=1#f" + it[name] = "new" + it[avatarUrl] = "b" + it[subscribedAt] = 20L + } + SubscriptionsTable.insert { + it[userId] = "other" + it[channelUrl] = "http://WWW.YouTube.com/channel/UC123/?x=1#f" + it[name] = "other" + it[avatarUrl] = "c" + it[subscribedAt] = 30L + } + DatabaseSubscriptionsCanonicalMigration.apply() + val rows = SubscriptionsTable.selectAll() + .where { SubscriptionsTable.userId eq TEST_USER_ID } + .orderBy(SubscriptionsTable.subscribedAt to SortOrder.DESC) + .toList() + assertEquals(1, rows.size) + assertEquals("https://www.youtube.com/channel/UC123", rows.first()[SubscriptionsTable.channelUrl]) + assertEquals("new", rows.first()[SubscriptionsTable.name]) + assertEquals(20L, rows.first()[SubscriptionsTable.subscribedAt]) + val otherRows = SubscriptionsTable.selectAll().where { SubscriptionsTable.userId eq "other" }.toList() + assertEquals(1, otherRows.size) + assertEquals("https://www.youtube.com/channel/UC123", otherRows.first()[SubscriptionsTable.channelUrl]) + } + } +} diff --git a/src/test/kotlin/dev/typetype/server/SubscriptionsCanonicalizationRoutesTest.kt b/src/test/kotlin/dev/typetype/server/SubscriptionsCanonicalizationRoutesTest.kt new file mode 100644 index 0000000..e819167 --- /dev/null +++ b/src/test/kotlin/dev/typetype/server/SubscriptionsCanonicalizationRoutesTest.kt @@ -0,0 +1,77 @@ +package dev.typetype.server + +import dev.typetype.server.models.SubscriptionItem +import dev.typetype.server.routes.subscriptionsRoutes +import dev.typetype.server.services.AuthService +import dev.typetype.server.services.SubscriptionsService +import io.ktor.client.request.delete +import io.ktor.client.request.get +import io.ktor.client.request.headers +import io.ktor.client.request.post +import io.ktor.client.request.setBody +import io.ktor.client.statement.bodyAsText +import io.ktor.http.ContentType +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +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 org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +class SubscriptionsCanonicalizationRoutesTest { + private val service = SubscriptionsService() + private val auth = AuthService.fixed(TEST_USER_ID) + + companion object { + @BeforeAll + @JvmStatic + fun initDb() = TestDatabase.setup() + } + + @BeforeEach + fun clean() { + TestDatabase.truncateAll() + } + + private fun withApp(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication { + application { + install(ContentNegotiation) { json() } + routing { subscriptionsRoutes(service, auth) } + } + block() + } + + @Test + fun `POST and GET subscriptions return canonical channel url`() = withApp { + val response = client.post("/subscriptions") { + headers.append(HttpHeaders.Authorization, "Bearer test-jwt") + headers.append(HttpHeaders.ContentType, ContentType.Application.Json.toString()) + setBody("""{"channelUrl":"http://WWW.YouTube.com/channel/UC123/?utm_source=x#frag","name":"Test","avatarUrl":""}""") + } + assertEquals(HttpStatusCode.Created, response.status) + assertTrue(response.bodyAsText().contains("\"channelUrl\":\"https://www.youtube.com/channel/UC123\"")) + val listBody = client.get("/subscriptions") { + headers.append(HttpHeaders.Authorization, "Bearer test-jwt") + }.bodyAsText() + assertTrue(listBody.contains("\"channelUrl\":\"https://www.youtube.com/channel/UC123\"")) + } + + @Test + fun `DELETE subscriptions canonicalizes channel url`() = withApp { + service.add( + TEST_USER_ID, + SubscriptionItem(channelUrl = "https://www.youtube.com/channel/UC123", name = "Test", avatarUrl = ""), + ) + val deleteResponse = client.delete("/subscriptions/http%3A%2F%2FWWW.YouTube.com%2Fchannel%2FUC123%2F%3Futm_source%3Dx%23frag") { + headers.append(HttpHeaders.Authorization, "Bearer test-jwt") + } + assertEquals(HttpStatusCode.NoContent, deleteResponse.status) + } +}