From 56fd40f8d7993f57886ab452296bd26a1b5a6aca Mon Sep 17 00:00:00 2001 From: CocoisBuggy Date: Fri, 9 May 2025 12:18:52 +0200 Subject: [PATCH] fix for #468 and partial solution to #413 I've added some e2e tests to make sure things are above board but there is a bit of potential uncertainty. Frustratingly, this is a part of the GQL architechture that leads to a growing number of utility types that developers need to keep track of. Because I do not have a good insight into how many services still depend on the old user profile resolutions I have left them untouched - though it could be argued they need to be deprecated. As far as resolving user nodes on Media, this is simply a danger we are going to accept. We'd like to PREVENT resolving greedily in places where it's not necessary but really there's nothing we can do to prevent fronent devs from doing silly things --- src/__tests__/media.e2e.ts | 159 +++++++++++++++++ src/__tests__/user.e2e.ts | 254 ++++++++++++++++++++++++++++ src/graphql/media/MediaResolvers.ts | 6 +- src/graphql/media/queries.ts | 3 +- src/graphql/schema/Media.gql | 1 + src/graphql/schema/User.gql | 18 +- src/graphql/user/UserQueries.ts | 43 +++++ src/model/UserDataSource.ts | 25 +++ 8 files changed, 504 insertions(+), 5 deletions(-) create mode 100644 src/__tests__/media.e2e.ts create mode 100644 src/__tests__/user.e2e.ts diff --git a/src/__tests__/media.e2e.ts b/src/__tests__/media.e2e.ts new file mode 100644 index 00000000..6f024511 --- /dev/null +++ b/src/__tests__/media.e2e.ts @@ -0,0 +1,159 @@ +import { ApolloServer } from '@apollo/server' +import muuid from 'uuid-mongodb' +import { jest } from '@jest/globals' +import MutableMediaDataSource from '../model/MutableMediaDataSource.js' +import { MediaObject, MediaObjectGQLInput } from '../db/MediaObjectTypes.js' +import { queryAPI, setUpServer } from '../utils/testUtils.js' +import { muuidToString } from '../utils/helpers.js' +import { InMemoryDB } from '../utils/inMemoryDB.js' +import express from 'express' +import UserDataSource from '../model/UserDataSource.js' + +jest.setTimeout(60000) + +describe('E2E tests to validate behavior of media queries', () => { + let server: ApolloServer + let user: muuid.MUUID + let userUuid: string + let app: express.Application + let inMemoryDB: InMemoryDB + let userDs: UserDataSource + + beforeAll(async () => { + ({ server, inMemoryDB, app } = await setUpServer()) + // Auth0 serializes uuids in "relaxed" mode, resulting in this hex string format + // "59f1d95a-627d-4b8c-91b9-389c7424cb54" instead of base64 "WfHZWmJ9S4yRuTicdCTLVA==". + user = muuid.mode('relaxed').v4() + userUuid = muuidToString(user) + }) + + beforeEach(async () => { + await inMemoryDB.clear() + userDs = UserDataSource.getInstance() + const res = await userDs.createOrUpdateUserProfile(user, { + userUuid, + username: 'iwannaclimbv17oneday', + email: 'gumby@openbeta.io', + displayName: 'jared' + }) + expect(res).toBe(true) + }) + + afterAll(async () => { + await server.stop() + await inMemoryDB.close() + }) + + async function insertMediaObjects (mediaCount: number): Promise { + const newMediaListInput: MediaObjectGQLInput[] = [] + for (let i = 0; i < mediaCount; i++) { + newMediaListInput.push({ + userUuid, + width: 800, + height: 600, + format: 'jpeg', + size: 45000, + mediaUrl: `/areaPhoto${i}.jpg` + }) + } + + const media = MutableMediaDataSource.getInstance() + return await media.addMediaObjects(newMediaListInput) + } + + describe('media queries', () => { + it('can resolve known media', async () => { + const [object] = await insertMediaObjects(1) + const response = await queryAPI({ + query: ` + query Media($input: MediaInput) { + media(input: $input) { + id + mediaUrl + } + } + `, + operationName: 'Media', + variables: { + input: { + id: object._id.toString() + } + }, + userUuid, + app + }) + + expect(response.statusCode).toBe(200) + expect(response.error).toBe(false) + const mediaResult = response.body.data.media + expect(mediaResult.id).toBe(object._id.toHexString()) + }) + + it('Media resolver can yield a simple username', async () => { + const [object] = await insertMediaObjects(1) + const response = await queryAPI({ + query: ` + query Media($input: MediaInput) { + media(input: $input) { + id + mediaUrl + username + } + } + `, + operationName: 'Media', + variables: { + input: { + id: object._id.toString() + } + }, + userUuid, + app + }) + + expect(response.statusCode).toBe(200) + expect(response.error).toBe(false) + console.log(response) + const mediaResult = response.body.data.media + expect(mediaResult.id).toBe(object._id.toHexString()) + expect(mediaResult.mediaUrl).toBeTruthy() + expect(mediaResult.username).not.toBeNull() + expect(mediaResult.username).not.toBeUndefined() + }) + + it('Media resolver can yield a user node', async () => { + const [object] = await insertMediaObjects(1) + const response = await queryAPI({ + query: ` + query Media($input: MediaInput) { + media(input: $input) { + id + mediaUrl + user { + username + } + } + } + `, + operationName: 'Media', + variables: { + input: { + id: object._id.toString() + } + }, + userUuid, + app + }) + + expect(response.statusCode).toBe(200) + expect(response.error).toBe(false) + console.log(response) + const mediaResult = response.body.data.media + expect(mediaResult.id).toBe(object._id.toHexString()) + expect(mediaResult.mediaUrl).toBeTruthy() + console.log(mediaResult) + expect(mediaResult.user).not.toBeNull() + expect(mediaResult.user.username).toBe('iwannaclimbv17oneday') + }) + }) +}) diff --git a/src/__tests__/user.e2e.ts b/src/__tests__/user.e2e.ts new file mode 100644 index 00000000..1bc0beb3 --- /dev/null +++ b/src/__tests__/user.e2e.ts @@ -0,0 +1,254 @@ +import { ApolloServer } from '@apollo/server' +import muuid from 'uuid-mongodb' +import { jest } from '@jest/globals' +import { queryAPI, setUpServer } from '../utils/testUtils.js' +import { muuidToString } from '../utils/helpers.js' +import { InMemoryDB } from '../utils/inMemoryDB.js' +import express from 'express' +import UserDataSource from '../model/UserDataSource.js' + +jest.setTimeout(60000) + +describe('E2E tests for user queries', () => { + let server: ApolloServer + let user: muuid.MUUID + let userUuid: string + let app: express.Application + let inMemoryDB: InMemoryDB + let userDs: UserDataSource + + beforeAll(async () => { + ({ server, inMemoryDB, app } = await setUpServer()) + // Auth0 serializes uuids in "relaxed" mode, resulting in this hex string format + // "59f1d95a-627d-4b8c-91b9-389c7424cb54" instead of base64 "WfHZWmJ9S4yRuTicdCTLVA==". + user = muuid.mode('relaxed').v4() + userUuid = muuidToString(user) + }) + + beforeEach(async () => { + await inMemoryDB.clear() + userDs = UserDataSource.getInstance() + const res = await userDs.createOrUpdateUserProfile(user, { + userUuid, + username: 'iwannaclimbv17oneday', + email: 'gumby@openbeta.io', + displayName: 'jared' + }) + + expect(res).toBe(true) + }) + + afterAll(async () => { + await server.stop() + await inMemoryDB.close() + }) + it('can resolve a user public profile by their uuid', async () => { + const response = await queryAPI({ + query: ` + query User($input: UserIDInput!) { + getUserPublicProfileByUuid(input: $input) { + userUuid + username + } + } + `, + operationName: 'User', + variables: { + input: { + userUuid + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.getUserPublicProfileByUuid + expect(user.username).toBe('iwannaclimbv17oneday') + }) + + describe('user query', () => { + const userQuery = ` + query User($input: LocateUserBy!) { + user(input: $input) { + userUuid + username + } + } + ` + + it('can resolve a user public profile by supplying username', async () => { + const response = await queryAPI({ + query: userQuery, + operationName: 'User', + variables: { + input: { + username: 'iwannaclimbv17oneday' + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.user + expect(user.username).toBe('iwannaclimbv17oneday') + }) + + it('can resolve a user public profile by supplying UUID', async () => { + const response = await queryAPI({ + query: userQuery, + operationName: 'User', + variables: { + input: { + userUuid + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.user + expect(user.username).toBe('iwannaclimbv17oneday') + }) + + it('will error when no input is given', async () => { + const response = await queryAPI({ + query: userQuery, + operationName: 'User', + variables: { + input: { + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeTruthy() + }) + + it('will prefer uuid if both are supplied', async () => { + const response = await queryAPI({ + query: userQuery, + operationName: 'User', + variables: { + input: { + userUuid, + username: 'some bunk name (**&%&*' + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.user + expect(user.username).toBe('iwannaclimbv17oneday') + }) + }) + + describe('page variants', () => { + const userPageQuery = ` + query User($input: LocateUserBy!) { + userPage(input: $input) { + profile { + userUuid + username + } + } + } + ` + + it('can resolve a user public page by supplying username', async () => { + const response = await queryAPI({ + query: userPageQuery, + operationName: 'User', + variables: { + input: { + username: 'iwannaclimbv17oneday' + } + }, + userUuid, + app + }) + + if (response.error !== false) { + throw response.error + } + + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.userPage.profile + expect(user.username).toBe('iwannaclimbv17oneday') + }) + + it('can resolve a user public page by supplying UUID', async () => { + const response = await queryAPI({ + query: userPageQuery, + operationName: 'User', + variables: { + input: { + userUuid + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.userPage.profile + expect(user.username).toBe('iwannaclimbv17oneday') + }) + + it('will error when no input is given', async () => { + const response = await queryAPI({ + query: userPageQuery, + operationName: 'User', + variables: { + input: { + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeTruthy() + }) + + it('will prefer uuid if both are supplied', async () => { + const response = await queryAPI({ + query: userPageQuery, + operationName: 'User', + variables: { + input: { + userUuid, + username: 'some bunk name (**&%&*' + } + }, + userUuid, + app + }) + + expect(response.error).toBe(false) + expect(response.statusCode).toBe(200) + expect(response.body.errors).toBeFalsy() + const user = response.body.data.userPage.profile + expect(user.username).toBe('iwannaclimbv17oneday') + }) + }) +}) diff --git a/src/graphql/media/MediaResolvers.ts b/src/graphql/media/MediaResolvers.ts index 8e031ab0..9725abe5 100644 --- a/src/graphql/media/MediaResolvers.ts +++ b/src/graphql/media/MediaResolvers.ts @@ -3,7 +3,6 @@ import { geojsonPointToLatitude, geojsonPointToLongitude } from '../../utils/hel import { DataSourcesType } from '../../types.js' const MediaResolvers = { - MediaByUsers: { userUuid: (node: MediaByUsers) => node.userUuid.toUUID().toString(), username: @@ -21,6 +20,11 @@ const MediaResolvers = { const u = await users.getUsername(node.userUuid) return u?.username ?? null }, + user: async (node: MediaObject, _: any, { dataSources }) => { + const { users } = dataSources as DataSourcesType + const u = await users.getUserPublicProfileByUuid(node.userUuid) + return u ?? null + }, uploadTime: (node: MediaObject) => node.createdAt }, diff --git a/src/graphql/media/queries.ts b/src/graphql/media/queries.ts index 1b53263e..23e1964c 100644 --- a/src/graphql/media/queries.ts +++ b/src/graphql/media/queries.ts @@ -2,8 +2,9 @@ import mongoose from 'mongoose' import muuid from 'uuid-mongodb' import { TagsLeaderboardType, MediaObject, MediaByUsers, UserMediaQueryInput, AreaMediaQueryInput, ClimbMediaQueryInput, MediaForFeedInput } from '../../db/MediaObjectTypes.js' import { GQLContext } from '../../types.js' +import { IResolvers } from '@graphql-tools/utils' -const MediaQueries = { +const MediaQueries: IResolvers = { media: async (_: any, { input }, { dataSources }: GQLContext): Promise => { const { media } = dataSources diff --git a/src/graphql/schema/Media.gql b/src/graphql/schema/Media.gql index 5e263de5..06835472 100644 --- a/src/graphql/schema/Media.gql +++ b/src/graphql/schema/Media.gql @@ -192,6 +192,7 @@ type EntityTag { type MediaWithTags implements IMediaMetadata { id: ID! username: String + user: UserPublicProfile mediaUrl: String! width: Int! height: Int! diff --git a/src/graphql/schema/User.gql b/src/graphql/schema/User.gql index 47ebe934..d26d3646 100644 --- a/src/graphql/schema/User.gql +++ b/src/graphql/schema/User.gql @@ -13,18 +13,30 @@ type Mutation { } type Query { + user(input: LocateUserBy!): UserPublicProfile! + userPage(input: LocateUserBy!): UserPublicPage! + "Check to see if a username already exists in the database." usernameExists(input: UsernameInput!): Boolean - "Get username object by user uuid" getUsername(input: UserIDInput!): UsernameDetail - "Get user public profile" getUserPublicProfileByUuid(input: UserIDInput!): UserPublicProfile - + "A users page is their profile + their media (paginated)" getUserPublicPage(input: UsernameInput!): UserPublicPage } +""" +Users can be principally identified either by their username or by their user id. +There is reason to prefer the latter over the former - usernames can be changed, and +in any scenario where you might be caching or trying to produce a permanent resource +reference you will want to prefer the uuid over the username. +""" +input LocateUserBy { + username: String + userUuid: ID +} + input UsernameInput { username: String! } diff --git a/src/graphql/user/UserQueries.ts b/src/graphql/user/UserQueries.ts index 2bde59ab..5bd9672c 100644 --- a/src/graphql/user/UserQueries.ts +++ b/src/graphql/user/UserQueries.ts @@ -5,6 +5,49 @@ import { DataSourcesType, ContextWithAuth, GQLContext } from '../../types.js' import { GetUsernameReturn, UserPublicProfile, UserPublicPage } from '../../db/UserTypes.js' const UserQueries = { + user: async (_: any, { input }, { dataSources }: ContextWithAuth): Promise => { + const { users }: DataSourcesType = dataSources + + let uuid = (input.userUuid !== undefined) && muuid.from(input.userUuid) + if (uuid === false) { + if (input.username === undefined) { + throw new Error('Supply either UUID (preferred) or username') + } + uuid = await users.uuidFromUsername(input.username) + } + + const profile = await users.getUserPublicProfileByUuid(uuid) + + if (profile === null) { + throw new Error('The requested user has no ascociated public profile') + } + + return profile + }, + + userPage: async (_: any, { input }, { dataSources }: ContextWithAuth): Promise => { + const { users, media: mediaDS }: DataSourcesType = dataSources + + let uuid = (input.userUuid !== undefined) && muuid.from(input.userUuid) + if (uuid === false) { + uuid = await users.uuidFromUsername(input.username) + } + + const profile = await users.getUserPublicProfileByUuid(uuid) + if (profile == null) { + throw new GraphQLError('User profile not found.', { + extensions: { + code: 'NOT_FOUND' + } + }) + } + + const media = await mediaDS.getOneUserMediaPagination({ userUuid: profile._id }) + return { + profile, + media + } + }, usernameExists: async (_: any, { input }, { dataSources }): Promise => { const { users }: DataSourcesType = dataSources diff --git a/src/model/UserDataSource.ts b/src/model/UserDataSource.ts index e6056c80..b9070eeb 100644 --- a/src/model/UserDataSource.ts +++ b/src/model/UserDataSource.ts @@ -219,6 +219,31 @@ export default class UserDataSource extends MongoDataSource { return await this.userModel.findOne({ _id: userUuid }, UserDataSource.PUBLIC_PROFILE_PROJECTION).lean() } + /** + * I don't have a solid insight into how many external services are still querying + * by username, so we support it as a failover keying strategy in the resolvers + */ + async uuidFromUsername (username: string): Promise { + if (typeof username !== 'string' || username === '') { + throw new Error('You must provide some positive username, or use a full UUID instead') + } + + if (!isValidUsername(username)) { + throw new Error('Invalid username') + } + + return (await this.userModel.findOne( + { + 'usernameInfo.username': { + $exists: true, $eq: username + } + }, + { + _id: 1 + } + ).orFail().lean())._id + } + /** * Get user profile data by user name * @param username username