From 6d8d23130d4b70d52437cce7152d39ab3e37d7eb Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 20 Jun 2024 06:44:06 -0400 Subject: [PATCH] Adding an image_details table to store image dimensions. (#4704) * Adding an image_details table to store image dimensions. - Adds an image_details table, which stores the height, width, and content_type for local and remote images. - For LocalImages, this information already comes back with the upload. - For RemoteImages, it calls the pictrs details endpoint. - Fixed some issues with proxying non-image urls. - Fixes #3328 - Also fixes #4703 * Running sql format. * Running fmt. * Don't fetch metadata in background for local API requests. * Dont export remote_image table to typescript. * Cleaning up validate. * Dont proxy url. * Fixing tests, fixing issue with federated thumbnails. * Fix tests. * Updating corepack, fixing issue. * Refactoring image inserts to use transactions. * Use select exists again. * Fixing imports. * Fix test. * Removing pointless backgrounded metadata generation version. * Removing public pictrs details route. * Fixing clippy. * Running prettier. * A few more fixes. * Moving diesel schema check back down. * Addressing PR comments. * Changing back request head to get. * Fixing lockfile. --------- Co-authored-by: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> --- api_tests/.eslintrc.json | 42 ----- api_tests/eslint.config.mjs | 56 ++++++ api_tests/package.json | 7 +- api_tests/pnpm-lock.yaml | 173 +++++++++++++++++- api_tests/prepare-drone-federation-test.sh | 2 +- api_tests/src/image.spec.ts | 4 + api_tests/src/post.spec.ts | 2 +- crates/api_common/src/request.rs | 58 +++++- crates/api_common/src/utils.rs | 32 +++- crates/db_schema/src/impls/images.rs | 78 ++++++-- crates/db_schema/src/schema.rs | 13 +- crates/db_schema/src/source/images.rs | 33 +++- crates/db_views/src/post_view.rs | 4 + crates/db_views/src/structs.rs | 3 +- crates/routes/src/images.rs | 8 +- .../down.sql | 7 + .../up.sql | 15 ++ 17 files changed, 453 insertions(+), 84 deletions(-) delete mode 100644 api_tests/.eslintrc.json create mode 100644 api_tests/eslint.config.mjs create mode 100644 migrations/2024-05-05-162540_add_image_detail_table/down.sql create mode 100644 migrations/2024-05-05-162540_add_image_detail_table/up.sql diff --git a/api_tests/.eslintrc.json b/api_tests/.eslintrc.json deleted file mode 100644 index 75b1706aa..000000000 --- a/api_tests/.eslintrc.json +++ /dev/null @@ -1,42 +0,0 @@ -{ - "root": true, - "env": { - "browser": true - }, - "plugins": ["@typescript-eslint"], - "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"], - "parser": "@typescript-eslint/parser", - "parserOptions": { - "project": "./tsconfig.json", - "warnOnUnsupportedTypeScriptVersion": false - }, - "rules": { - "@typescript-eslint/ban-ts-comment": 0, - "@typescript-eslint/no-explicit-any": 0, - "@typescript-eslint/explicit-module-boundary-types": 0, - "@typescript-eslint/no-var-requires": 0, - "arrow-body-style": 0, - "curly": 0, - "eol-last": 0, - "eqeqeq": 0, - "func-style": 0, - "import/no-duplicates": 0, - "max-statements": 0, - "max-params": 0, - "new-cap": 0, - "no-console": 0, - "no-duplicate-imports": 0, - "no-extra-parens": 0, - "no-return-assign": 0, - "no-throw-literal": 0, - "no-trailing-spaces": 0, - "no-unused-expressions": 0, - "no-useless-constructor": 0, - "no-useless-escape": 0, - "no-var": 0, - "prefer-const": 0, - "prefer-rest-params": 0, - "quote-props": 0, - "unicorn/filename-case": 0 - } -} diff --git a/api_tests/eslint.config.mjs b/api_tests/eslint.config.mjs new file mode 100644 index 000000000..cf2c426d0 --- /dev/null +++ b/api_tests/eslint.config.mjs @@ -0,0 +1,56 @@ +import pluginJs from "@eslint/js"; +import tseslint from "typescript-eslint"; + +export default [ + pluginJs.configs.recommended, + ...tseslint.configs.recommended, + { + languageOptions: { + parser: tseslint.parser, + }, + }, + // For some reason this has to be in its own block + { + ignores: [ + "putTypesInIndex.js", + "dist/*", + "docs/*", + ".yalc", + "jest.config.js", + ], + }, + { + files: ["src/**/*"], + rules: { + "@typescript-eslint/no-empty-interface": 0, + "@typescript-eslint/no-empty-function": 0, + "@typescript-eslint/ban-ts-comment": 0, + "@typescript-eslint/no-explicit-any": 0, + "@typescript-eslint/explicit-module-boundary-types": 0, + "@typescript-eslint/no-var-requires": 0, + "arrow-body-style": 0, + curly: 0, + "eol-last": 0, + eqeqeq: 0, + "func-style": 0, + "import/no-duplicates": 0, + "max-statements": 0, + "max-params": 0, + "new-cap": 0, + "no-console": 0, + "no-duplicate-imports": 0, + "no-extra-parens": 0, + "no-return-assign": 0, + "no-throw-literal": 0, + "no-trailing-spaces": 0, + "no-unused-expressions": 0, + "no-useless-constructor": 0, + "no-useless-escape": 0, + "no-var": 0, + "prefer-const": 0, + "prefer-rest-params": 0, + "quote-props": 0, + "unicorn/filename-case": 0, + }, + }, +]; diff --git a/api_tests/package.json b/api_tests/package.json index d80edfdc5..6a14bded7 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -8,7 +8,7 @@ "license": "AGPL-3.0", "packageManager": "pnpm@9.4.0", "scripts": { - "lint": "tsc --noEmit && eslint --report-unused-disable-directives --ext .js,.ts,.tsx src && prettier --check 'src/**/*.ts'", + "lint": "tsc --noEmit && eslint --report-unused-disable-directives && prettier --check 'src/**/*.ts'", "fix": "prettier --write src && eslint --fix src", "api-test": "jest -i follow.spec.ts && jest -i image.spec.ts && jest -i user.spec.ts && jest -i private_message.spec.ts && jest -i community.spec.ts && jest -i post.spec.ts && jest -i comment.spec.ts ", "api-test-follow": "jest -i follow.spec.ts", @@ -28,9 +28,10 @@ "eslint": "^9.0.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", - "lemmy-js-client": "0.19.4", + "lemmy-js-client": "0.19.5-alpha.1", "prettier": "^3.2.5", "ts-jest": "^29.1.0", - "typescript": "^5.4.4" + "typescript": "^5.4.4", + "typescript-eslint": "^7.13.0" } } diff --git a/api_tests/pnpm-lock.yaml b/api_tests/pnpm-lock.yaml index de2c9e84c..31ec952c1 100644 --- a/api_tests/pnpm-lock.yaml +++ b/api_tests/pnpm-lock.yaml @@ -33,8 +33,8 @@ importers: specifier: ^29.5.0 version: 29.7.0(@types/node@20.14.5) lemmy-js-client: - specifier: 0.19.4 - version: 0.19.4 + specifier: 0.19.5-alpha.1 + version: 0.19.5-alpha.1 prettier: specifier: ^3.2.5 version: 3.3.2 @@ -44,6 +44,9 @@ importers: typescript: specifier: ^5.4.4 version: 5.4.5 + typescript-eslint: + specifier: ^7.13.0 + version: 7.13.0(eslint@9.5.0)(typescript@5.4.5) packages: @@ -416,6 +419,17 @@ packages: '@types/yargs@17.0.32': resolution: {integrity: sha512-xQ67Yc/laOG5uMfX/093MRlGGCIBzZMarVa+gfNKJxWAIgykYpVGkBdbqEzGDDfCrVUj6Hiff4mTZ5BA6TmAog==} + '@typescript-eslint/eslint-plugin@7.13.0': + resolution: {integrity: sha512-FX1X6AF0w8MdVFLSdqwqN/me2hyhuQg4ykN6ZpVhh1ij/80pTvDKclX1sZB9iqex8SjQfVhwMKs3JtnnMLzG9w==} + engines: {node: ^18.18.0 || >=20.0.0} + peerDependencies: + '@typescript-eslint/parser': ^7.0.0 + eslint: ^8.56.0 + typescript: '*' + peerDependenciesMeta: + typescript: + optional: true + '@typescript-eslint/eslint-plugin@7.13.1': resolution: {integrity: sha512-kZqi+WZQaZfPKnsflLJQCz6Ze9FFSMfXrrIOcyargekQxG37ES7DJNpJUE9Q/X5n3yTIP/WPutVNzgknQ7biLg==} engines: {node: ^18.18.0 || >=20.0.0} @@ -427,6 +441,16 @@ packages: typescript: optional: true + '@typescript-eslint/parser@7.13.0': + resolution: {integrity: sha512-EjMfl69KOS9awXXe83iRN7oIEXy9yYdqWfqdrFAYAAr6syP8eLEFI7ZE4939antx2mNgPRW/o1ybm2SFYkbTVA==} + engines: {node: ^18.18.0 || >=20.0.0} + peerDependencies: + eslint: ^8.56.0 + typescript: '*' + peerDependenciesMeta: + typescript: + optional: true + '@typescript-eslint/parser@7.13.1': resolution: {integrity: sha512-1ELDPlnLvDQ5ybTSrMhRTFDfOQEOXNM+eP+3HT/Yq7ruWpciQw+Avi73pdEbA4SooCawEWo3dtYbF68gN7Ed1A==} engines: {node: ^18.18.0 || >=20.0.0} @@ -437,10 +461,24 @@ packages: typescript: optional: true + '@typescript-eslint/scope-manager@7.13.0': + resolution: {integrity: sha512-ZrMCe1R6a01T94ilV13egvcnvVJ1pxShkE0+NDjDzH4nvG1wXpwsVI5bZCvE7AEDH1mXEx5tJSVR68bLgG7Dng==} + engines: {node: ^18.18.0 || >=20.0.0} + '@typescript-eslint/scope-manager@7.13.1': resolution: {integrity: sha512-adbXNVEs6GmbzaCpymHQ0MB6E4TqoiVbC0iqG3uijR8ZYfpAXMGttouQzF4Oat3P2GxDVIrg7bMI/P65LiQZdg==} engines: {node: ^18.18.0 || >=20.0.0} + '@typescript-eslint/type-utils@7.13.0': + resolution: {integrity: sha512-xMEtMzxq9eRkZy48XuxlBFzpVMDurUAfDu5Rz16GouAtXm0TaAoTFzqWUFPPuQYXI/CDaH/Bgx/fk/84t/Bc9A==} + engines: {node: ^18.18.0 || >=20.0.0} + peerDependencies: + eslint: ^8.56.0 + typescript: '*' + peerDependenciesMeta: + typescript: + optional: true + '@typescript-eslint/type-utils@7.13.1': resolution: {integrity: sha512-aWDbLu1s9bmgPGXSzNCxELu+0+HQOapV/y+60gPXafR8e2g1Bifxzevaa+4L2ytCWm+CHqpELq4CSoN9ELiwCg==} engines: {node: ^18.18.0 || >=20.0.0} @@ -451,10 +489,23 @@ packages: typescript: optional: true + '@typescript-eslint/types@7.13.0': + resolution: {integrity: sha512-QWuwm9wcGMAuTsxP+qz6LBBd3Uq8I5Nv8xb0mk54jmNoCyDspnMvVsOxI6IsMmway5d1S9Su2+sCKv1st2l6eA==} + engines: {node: ^18.18.0 || >=20.0.0} + '@typescript-eslint/types@7.13.1': resolution: {integrity: sha512-7K7HMcSQIAND6RBL4kDl24sG/xKM13cA85dc7JnmQXw2cBDngg7c19B++JzvJHRG3zG36n9j1i451GBzRuHchw==} engines: {node: ^18.18.0 || >=20.0.0} + '@typescript-eslint/typescript-estree@7.13.0': + resolution: {integrity: sha512-cAvBvUoobaoIcoqox1YatXOnSl3gx92rCZoMRPzMNisDiM12siGilSM4+dJAekuuHTibI2hVC2fYK79iSFvWjw==} + engines: {node: ^18.18.0 || >=20.0.0} + peerDependencies: + typescript: '*' + peerDependenciesMeta: + typescript: + optional: true + '@typescript-eslint/typescript-estree@7.13.1': resolution: {integrity: sha512-uxNr51CMV7npU1BxZzYjoVz9iyjckBduFBP0S5sLlh1tXYzHzgZ3BR9SVsNed+LmwKrmnqN3Kdl5t7eZ5TS1Yw==} engines: {node: ^18.18.0 || >=20.0.0} @@ -464,12 +515,22 @@ packages: typescript: optional: true + '@typescript-eslint/utils@7.13.0': + resolution: {integrity: sha512-jceD8RgdKORVnB4Y6BqasfIkFhl4pajB1wVxrF4akxD2QPM8GNYjgGwEzYS+437ewlqqrg7Dw+6dhdpjMpeBFQ==} + engines: {node: ^18.18.0 || >=20.0.0} + peerDependencies: + eslint: ^8.56.0 + '@typescript-eslint/utils@7.13.1': resolution: {integrity: sha512-h5MzFBD5a/Gh/fvNdp9pTfqJAbuQC4sCN2WzuXme71lqFJsZtLbjxfSk4r3p02WIArOF9N94pdsLiGutpDbrXQ==} engines: {node: ^18.18.0 || >=20.0.0} peerDependencies: eslint: ^8.56.0 + '@typescript-eslint/visitor-keys@7.13.0': + resolution: {integrity: sha512-nxn+dozQx+MK61nn/JP+M4eCkHDSxSLDpgE3WcQo0+fkjEolnaB5jswvIKC4K56By8MMgIho7f1PVxERHEo8rw==} + engines: {node: ^18.18.0 || >=20.0.0} + '@typescript-eslint/visitor-keys@7.13.1': resolution: {integrity: sha512-k/Bfne7lrP7hcb7m9zSsgcBmo+8eicqqfNAJ7uUY+jkTFpKeH2FSkWpFRtimBxgkyvqfu9jTPRbYOvud6isdXA==} engines: {node: ^18.18.0 || >=20.0.0} @@ -1175,8 +1236,8 @@ packages: resolution: {integrity: sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==} engines: {node: '>=6'} - lemmy-js-client@0.19.4: - resolution: {integrity: sha512-k3d+YRDj3+JuuEP+nuEg27efR/e4m8oMk2BoC8jq9AnMrwSAKfsN2F2vG70Zke0amXtOclDZrCSHkIpNw99ikg==} + lemmy-js-client@0.19.5-alpha.1: + resolution: {integrity: sha512-GOhaiTQzrpwdmc3DFYemT2SmNmpuQJe2BWUms9QOzdYlkA1WZ0uu7axPE3s+T5OOxfy7K9Q2gsLe72dcVSlffw==} leven@3.1.0: resolution: {integrity: sha512-qsda+H8jTaUaN/x5vzW2rzc+8Rw4TAQ/4KjB46IwK5VH+IlVeeeje/EoZRpiXvIqjFgK84QffqPztGI3VBLG1A==} @@ -1548,6 +1609,16 @@ packages: resolution: {integrity: sha512-t0rzBq87m3fVcduHDUFhKmyyX+9eo6WQjZvf51Ea/M0Q7+T374Jp1aUiyUl0GKxp8M/OETVHSDvmkyPgvX+X2w==} engines: {node: '>=10'} + typescript-eslint@7.13.0: + resolution: {integrity: sha512-upO0AXxyBwJ4BbiC6CRgAJKtGYha2zw4m1g7TIVPSonwYEuf7vCicw3syjS1OxdDMTz96sZIXl3Jx3vWJLLKFw==} + engines: {node: ^18.18.0 || >=20.0.0} + peerDependencies: + eslint: ^8.56.0 + typescript: '*' + peerDependenciesMeta: + typescript: + optional: true + typescript@5.4.5: resolution: {integrity: sha512-vcI4UpRgg81oIRUFwR0WSIHKt11nJ7SAVlYNIu+QpqeyXP+gpQJy/Z4+F0aGxSE4MqwjyXvW/TzgkLAx2AGHwQ==} engines: {node: '>=14.17'} @@ -2119,6 +2190,24 @@ snapshots: dependencies: '@types/yargs-parser': 21.0.3 + '@typescript-eslint/eslint-plugin@7.13.0(@typescript-eslint/parser@7.13.0(eslint@9.5.0)(typescript@5.4.5))(eslint@9.5.0)(typescript@5.4.5)': + dependencies: + '@eslint-community/regexpp': 4.10.1 + '@typescript-eslint/parser': 7.13.0(eslint@9.5.0)(typescript@5.4.5) + '@typescript-eslint/scope-manager': 7.13.0 + '@typescript-eslint/type-utils': 7.13.0(eslint@9.5.0)(typescript@5.4.5) + '@typescript-eslint/utils': 7.13.0(eslint@9.5.0)(typescript@5.4.5) + '@typescript-eslint/visitor-keys': 7.13.0 + eslint: 9.5.0 + graphemer: 1.4.0 + ignore: 5.3.1 + natural-compare: 1.4.0 + ts-api-utils: 1.3.0(typescript@5.4.5) + optionalDependencies: + typescript: 5.4.5 + transitivePeerDependencies: + - supports-color + '@typescript-eslint/eslint-plugin@7.13.1(@typescript-eslint/parser@7.13.1(eslint@9.5.0)(typescript@5.4.5))(eslint@9.5.0)(typescript@5.4.5)': dependencies: '@eslint-community/regexpp': 4.10.1 @@ -2137,6 +2226,19 @@ snapshots: transitivePeerDependencies: - supports-color + '@typescript-eslint/parser@7.13.0(eslint@9.5.0)(typescript@5.4.5)': + dependencies: + '@typescript-eslint/scope-manager': 7.13.0 + '@typescript-eslint/types': 7.13.0 + '@typescript-eslint/typescript-estree': 7.13.0(typescript@5.4.5) + '@typescript-eslint/visitor-keys': 7.13.0 + debug: 4.3.5 + eslint: 9.5.0 + optionalDependencies: + typescript: 5.4.5 + transitivePeerDependencies: + - supports-color + '@typescript-eslint/parser@7.13.1(eslint@9.5.0)(typescript@5.4.5)': dependencies: '@typescript-eslint/scope-manager': 7.13.1 @@ -2150,11 +2252,28 @@ snapshots: transitivePeerDependencies: - supports-color + '@typescript-eslint/scope-manager@7.13.0': + dependencies: + '@typescript-eslint/types': 7.13.0 + '@typescript-eslint/visitor-keys': 7.13.0 + '@typescript-eslint/scope-manager@7.13.1': dependencies: '@typescript-eslint/types': 7.13.1 '@typescript-eslint/visitor-keys': 7.13.1 + '@typescript-eslint/type-utils@7.13.0(eslint@9.5.0)(typescript@5.4.5)': + dependencies: + '@typescript-eslint/typescript-estree': 7.13.0(typescript@5.4.5) + '@typescript-eslint/utils': 7.13.0(eslint@9.5.0)(typescript@5.4.5) + debug: 4.3.5 + eslint: 9.5.0 + ts-api-utils: 1.3.0(typescript@5.4.5) + optionalDependencies: + typescript: 5.4.5 + transitivePeerDependencies: + - supports-color + '@typescript-eslint/type-utils@7.13.1(eslint@9.5.0)(typescript@5.4.5)': dependencies: '@typescript-eslint/typescript-estree': 7.13.1(typescript@5.4.5) @@ -2167,8 +2286,25 @@ snapshots: transitivePeerDependencies: - supports-color + '@typescript-eslint/types@7.13.0': {} + '@typescript-eslint/types@7.13.1': {} + '@typescript-eslint/typescript-estree@7.13.0(typescript@5.4.5)': + dependencies: + '@typescript-eslint/types': 7.13.0 + '@typescript-eslint/visitor-keys': 7.13.0 + debug: 4.3.5 + globby: 11.1.0 + is-glob: 4.0.3 + minimatch: 9.0.4 + semver: 7.6.2 + ts-api-utils: 1.3.0(typescript@5.4.5) + optionalDependencies: + typescript: 5.4.5 + transitivePeerDependencies: + - supports-color + '@typescript-eslint/typescript-estree@7.13.1(typescript@5.4.5)': dependencies: '@typescript-eslint/types': 7.13.1 @@ -2184,6 +2320,17 @@ snapshots: transitivePeerDependencies: - supports-color + '@typescript-eslint/utils@7.13.0(eslint@9.5.0)(typescript@5.4.5)': + dependencies: + '@eslint-community/eslint-utils': 4.4.0(eslint@9.5.0) + '@typescript-eslint/scope-manager': 7.13.0 + '@typescript-eslint/types': 7.13.0 + '@typescript-eslint/typescript-estree': 7.13.0(typescript@5.4.5) + eslint: 9.5.0 + transitivePeerDependencies: + - supports-color + - typescript + '@typescript-eslint/utils@7.13.1(eslint@9.5.0)(typescript@5.4.5)': dependencies: '@eslint-community/eslint-utils': 4.4.0(eslint@9.5.0) @@ -2195,6 +2342,11 @@ snapshots: - supports-color - typescript + '@typescript-eslint/visitor-keys@7.13.0': + dependencies: + '@typescript-eslint/types': 7.13.0 + eslint-visitor-keys: 3.4.3 + '@typescript-eslint/visitor-keys@7.13.1': dependencies: '@typescript-eslint/types': 7.13.1 @@ -3078,7 +3230,7 @@ snapshots: kleur@3.0.3: {} - lemmy-js-client@0.19.4: {} + lemmy-js-client@0.19.5-alpha.1: {} leven@3.1.0: {} @@ -3387,6 +3539,17 @@ snapshots: type-fest@0.21.3: {} + typescript-eslint@7.13.0(eslint@9.5.0)(typescript@5.4.5): + dependencies: + '@typescript-eslint/eslint-plugin': 7.13.0(@typescript-eslint/parser@7.13.0(eslint@9.5.0)(typescript@5.4.5))(eslint@9.5.0)(typescript@5.4.5) + '@typescript-eslint/parser': 7.13.0(eslint@9.5.0)(typescript@5.4.5) + '@typescript-eslint/utils': 7.13.0(eslint@9.5.0)(typescript@5.4.5) + eslint: 9.5.0 + optionalDependencies: + typescript: 5.4.5 + transitivePeerDependencies: + - supports-color + typescript@5.4.5: {} undici-types@5.26.5: {} diff --git a/api_tests/prepare-drone-federation-test.sh b/api_tests/prepare-drone-federation-test.sh index 31eb111c2..65c4827d9 100755 --- a/api_tests/prepare-drone-federation-test.sh +++ b/api_tests/prepare-drone-federation-test.sh @@ -15,7 +15,7 @@ export LEMMY_TEST_FAST_FEDERATION=1 # by default, the persistent federation queu # pictrs setup if [ ! -f "api_tests/pict-rs" ]; then - curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.13/pict-rs-linux-amd64" -o api_tests/pict-rs + curl "https://git.asonix.dog/asonix/pict-rs/releases/download/v0.5.16/pict-rs-linux-amd64" -o api_tests/pict-rs chmod +x api_tests/pict-rs fi ./api_tests/pict-rs \ diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 123982e85..fe9470d18 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -160,6 +160,7 @@ test("Purge post, linked image removed", async () => { upload.url, ); expect(post.post_view.post.url).toBe(upload.url); + expect(post.post_view.image_details).toBeDefined(); // purge post const purgeForm: PurgePost = { @@ -184,6 +185,9 @@ test("Images in remote image post are proxied if setting enabled", async () => { const post = postRes.post_view.post; expect(post).toBeDefined(); + // Make sure it fetched the image details + expect(postRes.post_view.image_details).toBeDefined(); + // remote image gets proxied after upload expect( post.thumbnail_url?.startsWith( diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index cb45274c6..fe17bd979 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -502,7 +502,7 @@ test("Enforce site ban federation for local user", async () => { } let newAlphaUserJwt = await loginUser(alpha, alphaUserPerson.name); alphaUserHttp.setHeaders({ - Authorization: "Bearer " + newAlphaUserJwt.jwt ?? "", + Authorization: "Bearer " + newAlphaUserJwt.jwt, }); // alpha makes new post in beta community, it federates let postRes2 = await createPost(alphaUserHttp, betaCommunity!.community.id); diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index 8a423ff7c..ddbb3dd0c 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -11,7 +11,7 @@ use encoding_rs::{Encoding, UTF_8}; use lemmy_db_schema::{ newtypes::DbUrl, source::{ - images::{LocalImage, LocalImageForm}, + images::{ImageDetailsForm, LocalImage, LocalImageForm}, local_site::LocalSite, post::{Post, PostUpdateForm}, }, @@ -209,6 +209,19 @@ pub struct PictrsFileDetails { pub created_at: DateTime, } +impl PictrsFileDetails { + /// Builds the image form. This should always use the thumbnail_url, + /// Because the post_view joins to it + pub fn build_image_details_form(&self, thumbnail_url: &Url) -> ImageDetailsForm { + ImageDetailsForm { + link: thumbnail_url.clone().into(), + width: self.width.into(), + height: self.height.into(), + content_type: self.content_type.clone(), + } + } +} + #[derive(Deserialize, Serialize, Debug)] struct PictrsPurgeResponse { msg: String, @@ -316,11 +329,52 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L let protocol_and_hostname = context.settings().get_protocol_and_hostname(); let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; - LocalImage::create(&mut context.pool(), &form).await?; + // Also store the details for the image + let details_form = image.details.build_image_details_form(&thumbnail_url); + LocalImage::create(&mut context.pool(), &form, &details_form).await?; Ok(thumbnail_url) } +/// Fetches the image details for pictrs proxied images +/// +/// We don't need to check for image mode, as that's already been done +#[tracing::instrument(skip_all)] +pub async fn fetch_pictrs_proxied_image_details( + image_url: &Url, + context: &LemmyContext, +) -> LemmyResult { + let pictrs_url = context.settings().pictrs_config()?.url; + let encoded_image_url = encode(image_url.as_str()); + + // Pictrs needs you to fetch the proxied image before you can fetch the details + let proxy_url = format!("{pictrs_url}image/original?proxy={encoded_image_url}"); + + let res = context + .client() + .get(&proxy_url) + .timeout(REQWEST_TIMEOUT) + .send() + .await? + .status(); + if !res.is_success() { + Err(LemmyErrorType::NotAnImageType)? + } + + let details_url = format!("{pictrs_url}image/details/original?proxy={encoded_image_url}"); + + let res = context + .client() + .get(&details_url) + .timeout(REQWEST_TIMEOUT) + .send() + .await? + .json() + .await?; + + Ok(res) +} + // TODO: get rid of this by reading content type from db #[tracing::instrument(skip_all)] async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> LemmyResult<()> { diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index ba5756998..97b12cc5b 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -1,6 +1,10 @@ use crate::{ context::LemmyContext, - request::{delete_image_from_pictrs, purge_image_from_pictrs}, + request::{ + delete_image_from_pictrs, + fetch_pictrs_proxied_image_details, + purge_image_from_pictrs, + }, site::{FederatedInstances, InstanceWithFederationState}, }; use chrono::{DateTime, Days, Local, TimeZone, Utc}; @@ -949,7 +953,18 @@ pub async fn process_markdown( if context.settings().pictrs_config()?.image_mode() == PictrsImageMode::ProxyAllImages { let (text, links) = markdown_rewrite_image_links(text); - RemoteImage::create(&mut context.pool(), links).await?; + + // Create images and image detail rows + for link in links { + // Insert image details for the remote image + let details_res = fetch_pictrs_proxied_image_details(&link, context).await; + if let Ok(details) = details_res { + let proxied = + build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?; + let details_form = details.build_image_details_form(&proxied); + RemoteImage::create(&mut context.pool(), &details_form).await?; + } + } Ok(text) } else { Ok(text) @@ -984,8 +999,14 @@ async fn proxy_image_link_internal( Ok(link.into()) } else if image_mode == PictrsImageMode::ProxyAllImages { let proxied = build_proxied_image_url(&link, &context.settings().get_protocol_and_hostname())?; + // This should fail softly, since pictrs might not even be running + let details_res = fetch_pictrs_proxied_image_details(&link, context).await; + + if let Ok(details) = details_res { + let details_form = details.build_image_details_form(&proxied); + RemoteImage::create(&mut context.pool(), &details_form).await?; + }; - RemoteImage::create(&mut context.pool(), vec![link]).await?; Ok(proxied.into()) } else { Ok(link.into()) @@ -1123,10 +1144,13 @@ mod tests { "https://lemmy-alpha/api/v3/image_proxy?url=http%3A%2F%2Flemmy-beta%2Fimage.png", proxied.as_str() ); + + // This fails, because the details can't be fetched without pictrs running, + // And a remote image won't be inserted. assert!( RemoteImage::validate(&mut context.pool(), remote_image.into()) .await - .is_ok() + .is_err() ); } } diff --git a/crates/db_schema/src/impls/images.rs b/crates/db_schema/src/impls/images.rs index 9589aeee3..547bfc4e2 100644 --- a/crates/db_schema/src/impls/images.rs +++ b/crates/db_schema/src/impls/images.rs @@ -1,7 +1,14 @@ use crate::{ newtypes::DbUrl, - schema::{local_image, remote_image}, - source::images::{LocalImage, LocalImageForm, RemoteImage, RemoteImageForm}, + schema::{image_details, local_image, remote_image}, + source::images::{ + ImageDetails, + ImageDetailsForm, + LocalImage, + LocalImageForm, + RemoteImage, + RemoteImageForm, + }, utils::{get_conn, DbPool}, }; use diesel::{ @@ -13,15 +20,29 @@ use diesel::{ NotFound, QueryDsl, }; -use diesel_async::RunQueryDsl; -use url::Url; +use diesel_async::{AsyncPgConnection, RunQueryDsl}; impl LocalImage { - pub async fn create(pool: &mut DbPool<'_>, form: &LocalImageForm) -> Result { + pub async fn create( + pool: &mut DbPool<'_>, + form: &LocalImageForm, + image_details_form: &ImageDetailsForm, + ) -> Result { let conn = &mut get_conn(pool).await?; - insert_into(local_image::table) - .values(form) - .get_result::(conn) + conn + .build_transaction() + .run(|conn| { + Box::pin(async move { + let local_insert = insert_into(local_image::table) + .values(form) + .get_result::(conn) + .await; + + ImageDetails::create(conn, image_details_form).await?; + + local_insert + }) as _ + }) .await } @@ -39,16 +60,26 @@ impl LocalImage { } impl RemoteImage { - pub async fn create(pool: &mut DbPool<'_>, links: Vec) -> Result { + pub async fn create(pool: &mut DbPool<'_>, form: &ImageDetailsForm) -> Result { let conn = &mut get_conn(pool).await?; - let forms = links - .into_iter() - .map(|url| RemoteImageForm { link: url.into() }) - .collect::>(); - insert_into(remote_image::table) - .values(forms) - .on_conflict_do_nothing() - .execute(conn) + conn + .build_transaction() + .run(|conn| { + Box::pin(async move { + let remote_image_form = RemoteImageForm { + link: form.link.clone(), + }; + let remote_insert = insert_into(remote_image::table) + .values(remote_image_form) + .on_conflict_do_nothing() + .execute(conn) + .await; + + ImageDetails::create(conn, form).await?; + + remote_insert + }) as _ + }) .await } @@ -67,3 +98,16 @@ impl RemoteImage { } } } + +impl ImageDetails { + pub(crate) async fn create( + conn: &mut AsyncPgConnection, + form: &ImageDetailsForm, + ) -> Result { + insert_into(image_details::table) + .values(form) + .on_conflict_do_nothing() + .execute(conn) + .await + } +} diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 50bdac751..c3102b578 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -309,6 +309,15 @@ diesel::table! { } } +diesel::table! { + image_details (link) { + link -> Text, + width -> Int4, + height -> Int4, + content_type -> Text, + } +} + diesel::table! { instance (id) { id -> Int4, @@ -849,8 +858,7 @@ diesel::table! { } diesel::table! { - remote_image (id) { - id -> Int4, + remote_image (link) { link -> Text, published -> Timestamptz, } @@ -1055,6 +1063,7 @@ diesel::allow_tables_to_appear_in_same_query!( federation_allowlist, federation_blocklist, federation_queue_state, + image_details, instance, instance_block, language, diff --git a/crates/db_schema/src/source/images.rs b/crates/db_schema/src/source/images.rs index 9d48e011b..0dea4b84f 100644 --- a/crates/db_schema/src/source/images.rs +++ b/crates/db_schema/src/source/images.rs @@ -1,13 +1,12 @@ use crate::newtypes::{DbUrl, LocalUserId}; #[cfg(feature = "full")] -use crate::schema::{local_image, remote_image}; +use crate::schema::{image_details, local_image, remote_image}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; use std::fmt::Debug; #[cfg(feature = "full")] use ts_rs::TS; -use typed_builder::TypedBuilder; #[skip_serializing_none] #[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] @@ -30,7 +29,7 @@ pub struct LocalImage { pub published: DateTime, } -#[derive(Debug, Clone, TypedBuilder)] +#[derive(Debug, Clone)] #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = local_image))] pub struct LocalImageForm { @@ -46,15 +45,39 @@ pub struct LocalImageForm { #[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable))] #[cfg_attr(feature = "full", diesel(table_name = remote_image))] #[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] +#[cfg_attr(feature = "full", diesel(primary_key(link)))] pub struct RemoteImage { - pub id: i32, pub link: DbUrl, pub published: DateTime, } -#[derive(Debug, Clone, TypedBuilder)] +#[derive(Debug, Clone)] #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = remote_image))] pub struct RemoteImageForm { pub link: DbUrl, } + +#[skip_serializing_none] +#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] +#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable, TS))] +#[cfg_attr(feature = "full", ts(export))] +#[cfg_attr(feature = "full", diesel(table_name = image_details))] +#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] +#[cfg_attr(feature = "full", diesel(primary_key(link)))] +pub struct ImageDetails { + pub link: DbUrl, + pub width: i32, + pub height: i32, + pub content_type: String, +} + +#[derive(Debug, Clone)] +#[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] +#[cfg_attr(feature = "full", diesel(table_name = image_details))] +pub struct ImageDetailsForm { + pub link: DbUrl, + pub width: i32, + pub height: i32, + pub content_type: String, +} diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index fb616b0d3..d472fc604 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -28,6 +28,7 @@ use lemmy_db_schema::{ community_follower, community_moderator, community_person_ban, + image_details, instance_block, local_user, local_user_language, @@ -218,6 +219,7 @@ fn queries<'a>() -> Queries< .inner_join(person::table) .inner_join(community::table) .inner_join(post::table) + .left_join(image_details::table.on(post::thumbnail_url.eq(image_details::link.nullable()))) .left_join( post_saved::table.on( post_aggregates::post_id @@ -229,6 +231,7 @@ fn queries<'a>() -> Queries< post::all_columns, person::all_columns, community::all_columns, + image_details::all_columns.nullable(), is_creator_banned_from_community, is_local_user_banned_from_community_selection, creator_is_moderator, @@ -1631,6 +1634,7 @@ mod tests { public_key: inserted_person.public_key.clone(), last_refreshed_at: inserted_person.last_refreshed_at, }, + image_details: None, creator_banned_from_community: false, banned_from_community: false, creator_is_moderator: false, diff --git a/crates/db_views/src/structs.rs b/crates/db_views/src/structs.rs index 350e4cad4..3c219d63f 100644 --- a/crates/db_views/src/structs.rs +++ b/crates/db_views/src/structs.rs @@ -8,7 +8,7 @@ use lemmy_db_schema::{ community::Community, custom_emoji::CustomEmoji, custom_emoji_keyword::CustomEmojiKeyword, - images::LocalImage, + images::{ImageDetails, LocalImage}, local_site::LocalSite, local_site_rate_limit::LocalSiteRateLimit, local_user::LocalUser, @@ -131,6 +131,7 @@ pub struct PostView { pub post: Post, pub creator: Person, pub community: Community, + pub image_details: Option, pub creator_banned_from_community: bool, pub banned_from_community: bool, pub creator_is_moderator: bool, diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 96d7d317c..049bd6cc8 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -103,7 +103,13 @@ async fn upload( pictrs_alias: image.file.to_string(), pictrs_delete_token: image.delete_token.to_string(), }; - LocalImage::create(&mut context.pool(), &form).await?; + + let protocol_and_hostname = context.settings().get_protocol_and_hostname(); + let thumbnail_url = image.thumbnail_url(&protocol_and_hostname)?; + + // Also store the details for the image + let details_form = image.details.build_image_details_form(&thumbnail_url); + LocalImage::create(&mut context.pool(), &form, &details_form).await?; } } diff --git a/migrations/2024-05-05-162540_add_image_detail_table/down.sql b/migrations/2024-05-05-162540_add_image_detail_table/down.sql new file mode 100644 index 000000000..819277c7f --- /dev/null +++ b/migrations/2024-05-05-162540_add_image_detail_table/down.sql @@ -0,0 +1,7 @@ +ALTER TABLE remote_image + ADD UNIQUE (link), + DROP CONSTRAINT remote_image_pkey, + ADD COLUMN id serial PRIMARY KEY; + +DROP TABLE image_details; + diff --git a/migrations/2024-05-05-162540_add_image_detail_table/up.sql b/migrations/2024-05-05-162540_add_image_detail_table/up.sql new file mode 100644 index 000000000..9b2ed9658 --- /dev/null +++ b/migrations/2024-05-05-162540_add_image_detail_table/up.sql @@ -0,0 +1,15 @@ +-- Drop the id column from the remote_image table, just use link +ALTER TABLE remote_image + DROP COLUMN id, + ADD PRIMARY KEY (link), + DROP CONSTRAINT remote_image_link_key; + +-- No good way to do references here unfortunately, unless we combine the images tables +-- The link should be the URL, not the pictrs_alias, to allow joining from post.thumbnail_url +CREATE TABLE image_details ( + link text PRIMARY KEY, + width integer NOT NULL, + height integer NOT NULL, + content_type text NOT NULL +); +