From b38a4f117be82bd6f9e8f0a7e68185ae24c607f6 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 16 Oct 2019 22:44:44 +0000 Subject: [PATCH] [CORL-667] Constraint Directive (#2641) * feat: added constraints * fix: fixes bug with index creation - fixes #2612 * fix: added default resolver * fix: added defaultTo to handle defaulting --- .../graph/common/directives/constraint.ts | 45 ++++++++++++ .../graph/tenant/loaders/CommentActions.ts | 6 +- .../loaders/CommentModerationActions.ts | 10 ++- .../server/graph/tenant/loaders/Comments.ts | 73 ++++++------------- .../server/graph/tenant/loaders/Stories.ts | 5 +- src/core/server/graph/tenant/loaders/Users.ts | 11 +-- .../server/graph/tenant/resolvers/Comment.ts | 5 +- .../graph/tenant/resolvers/ModerationQueue.ts | 11 ++- src/core/server/graph/tenant/schema/index.ts | 12 ++- .../server/graph/tenant/schema/schema.graphql | 41 +++++++---- .../migrations/1569612830133_indexes.ts | 36 ++++++++- 11 files changed, 163 insertions(+), 92 deletions(-) create mode 100644 src/core/server/graph/common/directives/constraint.ts diff --git a/src/core/server/graph/common/directives/constraint.ts b/src/core/server/graph/common/directives/constraint.ts new file mode 100644 index 000000000..255801faa --- /dev/null +++ b/src/core/server/graph/common/directives/constraint.ts @@ -0,0 +1,45 @@ +import { + defaultFieldResolver, + GraphQLArgument, + GraphQLField, + GraphQLInterfaceType, + GraphQLObjectType, +} from "graphql"; +import { SchemaDirectiveVisitor } from "graphql-tools"; +import { isNumber } from "lodash"; + +export default class extends SchemaDirectiveVisitor { + public visitArgumentDefinition( + argument: GraphQLArgument, + details: { + field: GraphQLField; + objectType: GraphQLObjectType | GraphQLInterfaceType; + } + ) { + const originalResolver = details.field.resolve || defaultFieldResolver; + details.field.resolve = async (...resolveArgs) => { + const argName = argument.name; + const args = resolveArgs[1]; // (parent, args, context, info) + const valueToValidate = args[argName]; + + // Apply number validations. + if (isNumber(valueToValidate)) { + // Apply min validation. + if (isNumber(this.args.min)) { + if (valueToValidate < this.args.min) { + throw new Error("too short"); + } + } + + // Apply max validation. + if (isNumber(this.args.max)) { + if (valueToValidate > this.args.max) { + throw new Error("too long"); + } + } + } + + return originalResolver.apply(this, resolveArgs); + }; + } +} diff --git a/src/core/server/graph/tenant/loaders/CommentActions.ts b/src/core/server/graph/tenant/loaders/CommentActions.ts index 2cc278f0a..6f80d17a8 100644 --- a/src/core/server/graph/tenant/loaders/CommentActions.ts +++ b/src/core/server/graph/tenant/loaders/CommentActions.ts @@ -1,3 +1,5 @@ +import { defaultTo } from "lodash"; + import Context from "coral-server/graph/tenant/context"; import { CommentActionConnectionInput, @@ -5,9 +7,9 @@ import { } from "coral-server/models/action/comment"; export default (ctx: Context) => ({ - connection: ({ first = 10, after, filter }: CommentActionConnectionInput) => + connection: ({ first, after, filter }: CommentActionConnectionInput) => retrieveCommentActionConnection(ctx.mongo, ctx.tenant.id, { - first, + first: defaultTo(first, 10), after, filter, }), diff --git a/src/core/server/graph/tenant/loaders/CommentModerationActions.ts b/src/core/server/graph/tenant/loaders/CommentModerationActions.ts index 2fd8bbd66..96a74990f 100644 --- a/src/core/server/graph/tenant/loaders/CommentModerationActions.ts +++ b/src/core/server/graph/tenant/loaders/CommentModerationActions.ts @@ -1,3 +1,5 @@ +import { defaultTo } from "lodash"; + import TenantContext from "coral-server/graph/tenant/context"; import { CommentToStatusHistoryArgs, @@ -7,22 +9,22 @@ import { retrieveCommentModerationActionConnection } from "coral-server/models/a export default (ctx: TenantContext) => ({ forModerator: ( - { first = 10, after }: UserToCommentModerationActionHistoryArgs, + { first, after }: UserToCommentModerationActionHistoryArgs, moderatorID: string ) => retrieveCommentModerationActionConnection(ctx.mongo, ctx.tenant.id, { - first, + first: defaultTo(first, 10), after, filter: { moderatorID, }, }), forComment: ( - { first = 10, after }: CommentToStatusHistoryArgs, + { first, after }: CommentToStatusHistoryArgs, commentID: string ) => retrieveCommentModerationActionConnection(ctx.mongo, ctx.tenant.id, { - first, + first: defaultTo(first, 10), after, filter: { commentID, diff --git a/src/core/server/graph/tenant/loaders/Comments.ts b/src/core/server/graph/tenant/loaders/Comments.ts index a8e4c4c08..01188d5e5 100644 --- a/src/core/server/graph/tenant/loaders/Comments.ts +++ b/src/core/server/graph/tenant/loaders/Comments.ts @@ -1,5 +1,5 @@ import DataLoader from "dataloader"; -import { isNil, omitBy } from "lodash"; +import { defaultTo, isNil, omitBy } from "lodash"; import { DateTime } from "luxon"; import Context from "coral-server/graph/tenant/context"; @@ -126,7 +126,7 @@ export default (ctx: Context) => ({ } ), forFilter: ({ - first = 10, + first, after, storyID, status, @@ -134,7 +134,7 @@ export default (ctx: Context) => ({ query, }: QueryToCommentsArgs) => retrieveCommentConnection(ctx.mongo, ctx.tenant.id, { - first, + first: defaultTo(first, 10), after, orderBy: GQLCOMMENT_SORT.CREATED_AT_DESC, filter: omitBy( @@ -164,71 +164,45 @@ export default (ctx: Context) => ({ ); } ), - forUser: ( - userID: string, - // Apply the graph schema defaults at the loader. - { - first = 10, - orderBy = GQLCOMMENT_SORT.CREATED_AT_DESC, - after, - }: UserToCommentsArgs - ) => + forUser: (userID: string, { first, orderBy, after }: UserToCommentsArgs) => retrieveCommentUserConnection(ctx.mongo, ctx.tenant.id, userID, { - first, - orderBy, + first: defaultTo(first, 10), + orderBy: defaultTo(orderBy, GQLCOMMENT_SORT.CREATED_AT_DESC), after, }).then(primeCommentsFromConnection(ctx)), - forUserAll: ( - userID: string, - // Apply the graph schema defaults at the loader. - { first = 10, after }: UserToAllCommentsArgs - ) => + forUserAll: (userID: string, { first, after }: UserToAllCommentsArgs) => retrieveAllCommentsUserConnection(ctx.mongo, ctx.tenant.id, userID, { - first, + first: defaultTo(first, 10), orderBy: GQLCOMMENT_SORT.CREATED_AT_DESC, after, }).then(primeCommentsFromConnection(ctx)), forUserRejected: ( userID: string, - // Apply the graph schema defaults at the loader. - { first = 10, after }: UserToRejectedCommentsArgs + { first, after }: UserToRejectedCommentsArgs ) => retrieveRejectedCommentUserConnection(ctx.mongo, ctx.tenant.id, userID, { - first, + first: defaultTo(first, 10), orderBy: GQLCOMMENT_SORT.CREATED_AT_DESC, after, }).then(primeCommentsFromConnection(ctx)), taggedForStory: ( storyID: string, tag: GQLTAG, - // Apply the graph schema defaults at the loader. - { - first = 10, - orderBy = GQLCOMMENT_SORT.CREATED_AT_DESC, - after, - }: StoryToCommentsArgs + { first, orderBy, after }: StoryToCommentsArgs ) => retrieveCommentStoryConnection(ctx.mongo, ctx.tenant.id, storyID, { - first, - orderBy, + first: defaultTo(first, 10), + orderBy: defaultTo(orderBy, GQLCOMMENT_SORT.CREATED_AT_DESC), after, filter: { // Filter optionally for comments with a specific tag. "tags.type": tag, }, }).then(primeCommentsFromConnection(ctx)), - forStory: ( - storyID: string, - // Apply the graph schema defaults at the loader. - { - first = 10, - orderBy = GQLCOMMENT_SORT.CREATED_AT_DESC, - after, - }: StoryToCommentsArgs - ) => + forStory: (storyID: string, { first, orderBy, after }: StoryToCommentsArgs) => retrieveCommentStoryConnection(ctx.mongo, ctx.tenant.id, storyID, { - first, - orderBy, + first: defaultTo(first, 10), + orderBy: defaultTo(orderBy, GQLCOMMENT_SORT.CREATED_AT_DESC), after, filter: { // Only get Comments that are top level. If the client wants to load @@ -239,12 +213,7 @@ export default (ctx: Context) => ({ forParent: ( storyID: string, parentID: string, - // Apply the graph schema defaults at the loader. - { - first = 10, - orderBy = GQLCOMMENT_SORT.CREATED_AT_DESC, - after, - }: CommentToRepliesArgs + { first, orderBy, after }: CommentToRepliesArgs ) => retrieveCommentRepliesConnection( ctx.mongo, @@ -252,14 +221,14 @@ export default (ctx: Context) => ({ storyID, parentID, { - first, - orderBy, + first: defaultTo(first, 10), + orderBy: defaultTo(orderBy, GQLCOMMENT_SORT.CREATED_AT_DESC), after, } ).then(primeCommentsFromConnection(ctx)), - parents: (comment: Comment, { last = 1, before }: CommentToParentsArgs) => + parents: (comment: Comment, { last, before }: CommentToParentsArgs) => retrieveCommentParentsConnection(ctx.mongo, ctx.tenant.id, comment, { - last, + last: defaultTo(last, 1), // The cursor passed here is always going to be a number. before: before as number, }).then(primeCommentsFromConnection(ctx)), diff --git a/src/core/server/graph/tenant/loaders/Stories.ts b/src/core/server/graph/tenant/loaders/Stories.ts index c610906a1..7cd5b1da7 100644 --- a/src/core/server/graph/tenant/loaders/Stories.ts +++ b/src/core/server/graph/tenant/loaders/Stories.ts @@ -1,4 +1,5 @@ import DataLoader from "dataloader"; +import { defaultTo } from "lodash"; import TenantContext from "coral-server/graph/tenant/context"; import { @@ -99,9 +100,9 @@ export default (ctx: TenantContext) => ({ cache: !ctx.disableCaching, } ), - connection: ({ first = 10, after, status, query }: QueryToStoriesArgs) => + connection: ({ first, after, status, query }: QueryToStoriesArgs) => retrieveStoryConnection(ctx.mongo, ctx.tenant.id, { - first, + first: defaultTo(first, 10), after, filter: { // Merge the status filter into the connection filter. diff --git a/src/core/server/graph/tenant/loaders/Users.ts b/src/core/server/graph/tenant/loaders/Users.ts index 0189ea50c..a1ecffcf0 100644 --- a/src/core/server/graph/tenant/loaders/Users.ts +++ b/src/core/server/graph/tenant/loaders/Users.ts @@ -1,4 +1,5 @@ import DataLoader from "dataloader"; +import { defaultTo } from "lodash"; import Context from "coral-server/graph/tenant/context"; import { @@ -112,15 +113,9 @@ export default (ctx: Context) => { return { user, - connection: ({ - first = 10, - after, - role, - query, - status, - }: QueryToUsersArgs) => + connection: ({ first, after, role, query, status }: QueryToUsersArgs) => retrieveUserConnection(ctx.mongo, ctx.tenant.id, { - first, + first: defaultTo(first, 10), after, filter: { // Merge the role filters into the query. diff --git a/src/core/server/graph/tenant/resolvers/Comment.ts b/src/core/server/graph/tenant/resolvers/Comment.ts index 54a543e25..e75e1f319 100644 --- a/src/core/server/graph/tenant/resolvers/Comment.ts +++ b/src/core/server/graph/tenant/resolvers/Comment.ts @@ -1,4 +1,5 @@ import { GraphQLResolveInfo } from "graphql"; +import { defaultTo } from "lodash"; import { StoryNotFoundError } from "coral-server/errors"; import { getRequestedFields } from "coral-server/graph/tenant/resolvers/util"; @@ -83,9 +84,9 @@ export const Comment: GQLCommentTypeResolver = { }, // Action Counts are encoded, decode them for use with the GraphQL system. actionCounts: c => decodeActionCounts(c.actionCounts), - flags: ({ id }, { first = 10, after }, ctx) => + flags: ({ id }, { first, after }, ctx) => ctx.loaders.CommentActions.connection({ - first, + first: defaultTo(first, 10), after, filter: { actionType: ACTION_TYPE.FLAG, diff --git a/src/core/server/graph/tenant/resolvers/ModerationQueue.ts b/src/core/server/graph/tenant/resolvers/ModerationQueue.ts index 138108271..e4b4ab63c 100644 --- a/src/core/server/graph/tenant/resolvers/ModerationQueue.ts +++ b/src/core/server/graph/tenant/resolvers/ModerationQueue.ts @@ -1,3 +1,5 @@ +import { defaultTo } from "lodash"; + import { CommentConnectionInput, retrieveCommentConnection, @@ -24,11 +26,12 @@ export const ModerationQueue: GQLModerationQueueTypeResolver< return selector; }, - comments: ({ connection }, { first = 10, after }, { mongo, tenant }) => - retrieveCommentConnection(mongo, tenant.id, { + comments: ({ connection }, { first, after }, { mongo, tenant }) => { + return retrieveCommentConnection(mongo, tenant.id, { ...connection, - first, + first: defaultTo(first, 10), after, orderBy: GQLCOMMENT_SORT.CREATED_AT_DESC, - }), + }); + }, }; diff --git a/src/core/server/graph/tenant/schema/index.ts b/src/core/server/graph/tenant/schema/index.ts index c9f09982a..923b1745b 100644 --- a/src/core/server/graph/tenant/schema/index.ts +++ b/src/core/server/graph/tenant/schema/index.ts @@ -1,7 +1,12 @@ -import { attachDirectiveResolvers, IResolvers } from "graphql-tools"; +import { + attachDirectiveResolvers, + IResolvers, + SchemaDirectiveVisitor, +} from "graphql-tools"; import { loadSchema } from "coral-common/graphql"; import auth from "coral-server/graph/common/directives/auth"; +import constraint from "coral-server/graph/common/directives/constraint"; import resolvers from "coral-server/graph/tenant/resolvers"; export default function getTenantSchema() { @@ -10,5 +15,10 @@ export default function getTenantSchema() { // Attach the directive resolvers. attachDirectiveResolvers(schema, { auth }); + // Attach the constraint directive. + SchemaDirectiveVisitor.visitSchemaDirectives(schema, { + constraint, + }); + return schema; } diff --git a/src/core/server/graph/tenant/schema/schema.graphql b/src/core/server/graph/tenant/schema/schema.graphql index 554b2bbbc..14b350b48 100644 --- a/src/core/server/graph/tenant/schema/schema.graphql +++ b/src/core/server/graph/tenant/schema/schema.graphql @@ -54,6 +54,12 @@ directive @auth( permit: [USER_AUTH_CONDITIONS!] ) on FIELD_DEFINITION +""" +constraint is a directive that will perform validation on input fields used as +arguments to parameters passed in to operations. +""" +directive @constraint(min: Int, max: Int) on ARGUMENT_DEFINITION + ################################################################################ ## Custom Scalar Types ################################################################################ @@ -372,7 +378,10 @@ type ModerationQueue { """ comments are the comments on the ModerationQueue. """ - comments(first: Int = 10, after: Cursor): CommentsConnection! + comments( + first: Int = 10 @constraint(max: 50) + after: Cursor + ): CommentsConnection! } """ @@ -1753,7 +1762,7 @@ type User { comments are the comments written by the User. """ comments( - first: Int = 10 + first: Int = 10 @constraint(max: 50) orderBy: COMMENT_SORT = CREATED_AT_DESC after: Cursor ): CommentsConnection! @@ -1766,14 +1775,18 @@ type User { """ allComments are comments regardless of visibility status. """ - allComments(first: Int = 10, after: Cursor): CommentsConnection! - @auth(roles: [ADMIN, MODERATOR]) + allComments( + first: Int = 10 @constraint(max: 50) + after: Cursor + ): CommentsConnection! @auth(roles: [ADMIN, MODERATOR]) """ rejectedComments are comments that have been rejected. """ - rejectedComments(first: Int = 10, after: Cursor): CommentsConnection! - @auth(roles: [ADMIN, MODERATOR]) + rejectedComments( + first: Int = 10 @constraint(max: 50) + after: Cursor + ): CommentsConnection! @auth(roles: [ADMIN, MODERATOR]) """ recentCommentHistory returns recent commenting history by the User. @@ -1785,7 +1798,7 @@ type User { that this User has created. """ commentModerationActionHistory( - first: Int = 10 + first: Int = 10 @constraint(max: 50) after: Cursor ): CommentModerationActionConnection! @auth(roles: [MODERATOR, ADMIN]) @@ -2127,7 +2140,7 @@ type Comment { recent last. """ statusHistory( - first: Int = 10 + first: Int = 10 @constraint(max: 50) after: Cursor ): CommentModerationActionConnection! @auth(roles: [MODERATOR, ADMIN]) @@ -2187,7 +2200,7 @@ type Comment { """ flags is the actual Flags that were left by the Users or the system. """ - flags(first: Int = 10, after: Cursor): FlagsConnection! + flags(first: Int = 10 @constraint(max: 50), after: Cursor): FlagsConnection! @auth(roles: [ADMIN, MODERATOR]) """ @@ -2466,7 +2479,7 @@ type Story { featuredComments are the Comments with the FEATURED tag on the Story. """ featuredComments( - first: Int = 10 + first: Int = 10 @constraint(max: 50) orderBy: COMMENT_SORT = CREATED_AT_DESC after: Cursor ): CommentsConnection! @@ -2475,7 +2488,7 @@ type Story { comments are the comments on the Story. """ comments( - first: Int = 10 + first: Int = 10 @constraint(max: 50) orderBy: COMMENT_SORT = CREATED_AT_DESC after: Cursor ): CommentsConnection! @@ -2571,7 +2584,7 @@ type Query { the dedicated edges for more optimized responses. """ comments( - first: Int = 10 + first: Int = 10 @constraint(max: 50) after: Cursor storyID: ID status: COMMENT_STATUS @@ -2588,7 +2601,7 @@ type Query { stories returns filtered stories that can be paginated. """ stories( - first: Int = 10 + first: Int = 10 @constraint(max: 50) after: Cursor status: STORY_STATUS query: String @@ -2607,7 +2620,7 @@ type Query { TODO: evaluate adding status based filtering. """ users( - first: Int = 10 + first: Int = 10 @constraint(max: 50) after: Cursor role: USER_ROLE query: String diff --git a/src/core/server/services/migrate/migrations/1569612830133_indexes.ts b/src/core/server/services/migrate/migrations/1569612830133_indexes.ts index 82c458ecc..e519452c7 100644 --- a/src/core/server/services/migrate/migrations/1569612830133_indexes.ts +++ b/src/core/server/services/migrate/migrations/1569612830133_indexes.ts @@ -1,4 +1,4 @@ -import { Db } from "mongodb"; +import { Db, MongoError } from "mongodb"; import Migration from "coral-server/services/migrate/migration"; import collections from "coral-server/services/mongodb/collections"; @@ -331,9 +331,39 @@ export default class extends Migration { } public async indexes(mongo: Db) { + // Find all the collections that exist already. + const results = await mongo + .listCollections({}, { nameOnly: true }) + .toArray(); + + const collectionNames = results + .filter(({ type }) => type === "collection") + .map(({ name }) => name); + // Drop existing indexes on managed collections so we can re-create them. - for (const collection of Object.values(collections)) { - await collection(mongo).dropIndexes(); + for (const collectionName in collections) { + if (!collections.hasOwnProperty(collectionName)) { + continue; + } + + // Check to see if this collection exists. + if (!collectionNames.includes(collectionName)) { + continue; + } + + try { + await mongo.collection(collectionName).dropIndexes(); + } catch (err) { + if (err instanceof MongoError) { + // If we're dropping indexes on a collection that doesn't exist, then + // don't worry. + if (err.code === 26) { + continue; + } + } + + throw err; + } } // Re-create the indexes for each collection now.