Ideas for parsing/validating/transforming graphql arguments according to business logic

[xoldd]

The graphql arguments need to be parsed/validated/transformed to be used in resolvers. During the validation step, errors will be encountered which will need to be relayed back to clients using type-safe errors within schema approach. So, something like utility functions. Take a reference from these discussions link1, link2.

The ideas need to incorporate arguments for all kinds of resolvers, whether they return a scalar field, a complex object, or the graphql connections. The ideas need to consider code resusability, correctness, and flexible result communication, and of course type-safety in mind.

A good starting point would be discussion on handling basic graphql connection arguments which are common to all graphql connection resolvers. The common arguments are first, after, last and before. Though additional arguments can be added to connection resolvers for purposes of sorting/filtering, but sorting/filtering will usually be different for different resolvers, so each connection resolver will probably need seperate method for parsing/validating/filtering sort/filter arguments.

[EshaanAgg]

Alright. This might be the most fun I have ever had with typescript and generalization.

For the purpose of implementation of cursor-based pagination, the basic redundancy in the code stems from three major places:

1. The implementation of the resolvers
2. The testing of the said resolvers
3. The error handling in the said resolvers

I have devised a general enough solution for the first problem that would allow the other two to be mitigated easily. We can start by extending the graphqlConnectionFactory to have a function that would take inputs from the user and generate the corresponding connection object by making all the relevant database requests.

Here is what the call signature of the function would look like:

type GetNodeFromResultFnType<T, U> = {
  (result: U): T;
};

type GetCursorFromResultFnType<U> = {
  (result: U): string;
};

type AfterFilterQueryType = {
  [key: string]: {
    $gte: string;
  };
};

type BeforeFilterQueryType = {
  [key: string]: {
    $lte: string;
  };
};

/*
This is a function which generates a GraphQL cursor pagination resolver based on the requirements of the client.

The function takes the following arguments: 

A. TYPE PARAMETERS

1. T: Refers the type of the node that the connection and it's edges would refer. 
Example values include `Interface_User`, `Interface_Organization`, `Interface_Event`, `Interface_Post`.
2. U: Regers to the type of interface that is implemented by the model that you want to query. 
For example, if you want to query the TagUser Model, then you would send Interface_UserTag for this type parameter

B. DATA PARAMETERS

1. args: These are the actual args that are send by the client in the request.
2. databaseModel: Refers to the actual database model that you want to query.
3. fieldToSortBy: Refers to the field on the Type U that you want to act as the cursor (ansd thus would be used
  for filtering and sorting or results). Can use the . notation to access nested values
4. filterQuery: Refers to the filter object that you want to pass to the .find() query which quering the databaseModel 
For example, User, Tag, Post, Organization etc.
5. fieldsToPopulate: A string that lists all the fields that you want to be populated in the model.
  It is an optional parameter and can be skipped.
6. getNodeFromResult: Describes a transformation function that given an object of type U, would convert it to the desired object of type T. This would mostly include mapping to some specific field of the fetched object.
7. getCursorFromResult: Describes a transformation function that is used to generate the cursor from a particular fetched 
  result object of type U. This would mostly be a mapping function. It must be noted that this field should exactly match
  the field provided in the fieldToSortBy argument to get sensible results.

It is important to know that the function would would sequentially in the following manner:
1. Fetch all the documents specified by your filter query.
2. Sort them by the field provided.
3. Populate the fields provided.
4. Run the functions getNodeFromResult and getCursorFromResult on each of the fetched objects from the database.

The function returns a promise which would resolve to the desired connection object (of the type Interface_Connection<T>).
*/
export async function createGraphQLConnection<T, U>(
  args: CursorPaginationArgsType,
  databaseModel: Model<U>,
  fieldToSortBy: string,
  filterQuery: FilterQuery<U>,
  fieldsToPopulate: string | null,
  getNodeFromResult: GetNodeFromResultFnType<T, U>,
  getCursorFromResult: GetCursorFromResultFnType<U>
): Promise<Interface_Connection<T>> {
  // Actual implementation of the function
}

I have tried to document the definitions and the use cases of the arguments as much as I can in the comments itself, but a few examples would go a long way in clarifying the use case!

1. usersAssignedTo resolver for UserTag model (where we treat the _id of the UserTag document as the cursor)

export const usersAssignedTo: UserTagResolvers["usersAssignedTo"] = async (
  parent,
  args
) => {
  return await createGraphQLConnection<Interface_User, Interface_TagUser>(
    args,
    TagUser,
    "_id",
    {
      tagId: parent._id,
    },
    "userId",
    (result) => result.userId,
    (result) => result._id.toString()
  );
};

Here,
we are querying on the TagUser model and using the _id field on it as the cursor. The {tagId: parent._id} is a usual filterQuery that is used to filter out the fetched documents from the database, and userId is the field that we want to be populated.
At last, we have defined two arrow functions, one mapping the fetched database object to the userId field (which is now populated with the whole User object and thus represents our node) and the second mapping the result object to the cursor.

2. usersAssignedTo resolver for UserTag model (where we treat the userId of the UserTag document as the cursor)

export const usersAssignedTo: UserTagResolvers["usersAssignedTo"] = async (
  parent,
  args
) => {
  return await createGraphQLConnection<Interface_User, Interface_TagUser>(
    args,
    TagUser,
    "userId",
    {
      tagId: parent._id,
    },
    "userId",
    (result) => result.userId,
    (result) => result.userId._id.toString()
  );
};

Here,
we are querying on the TagUser model and using the userId field on it as the cursor. The {tagId: parent._id} is a usual filterQuery that is used to filter out the fetched documents from the database, and userId is the field that we want to be populated.
At last, we have defined two arrow functions, one mapping the fetched database object to the userId field (which is now populated with the whole User object and thus represents our node) and the second mapping the result object to the cursor (the _id of the populated User Object).

3. childTags resolver for the UserTag model

export const childTags: UserTagResolvers["childTags"] = async (
  parent,
  args
) => {
  return await createGraphQLConnection<
    Interface_OrganizationTagUser,
    Interface_OrganizationTagUser
  >(
    args,
    OrganizationTagUser,
    "_id",
    {
      parentTagId: parent._id,
    },
    null,
    (result) => result,
    (result) => result._id.toString()
  );
};

Here,
we are querying on the OrganizationTagUser model and using the _id field on it as the cursor.
The {parentTagId: parent._id} is a usual filterQuery that is used to ensure that only the tag objects with the parent tag object as their parent tag are fetched from the database. No fields to be populated in this implementation, so we supply null.
At last, we have defined two arrow functions, one mapping the fetched database object to itself (as it already represents a tag object we want to return) and the second mapping the result object to the cursor (the _id of the fetched object).

As evident from the three examples, the presence of the said arguments makes the whole approach of generating pagination flexible enough so that it can be extended to all types of models and interfaces.

[kb-0311]

@xoldyckk @EshaanAgg Btw what are your thoughts on the current schema design in talawa-api? I mean resolving all the interrelated queries within a single root resolver using the populate function even if the client does not request that data. That is so REST-like so what is the point of even using graphs? I mean look at some of the mongoose populations in some of these queries, it is nuts.

In all examples online I have read, query resolvers should have the same name as a field so that it is resolved if and only if when that field is asked by the client specifically.But currently it seems that the API is resolving interrelated data everytime with populating in the root resolver when in reality those attributes should have its own resolver.

I had talked about @xoldyckk about this thing too.

Currently, to resolve relations we are relying on the mongoose's populate() However first thing to understand about mongoose population is that it is not magic, but just a convenience method that allows you to retrieve related information without doing it all yourself.

The "not magic" part is that essentially what happens under the covers is that when you "reference" another source, the populate function makes an additional query/queries to that "related" collection in order to "merge" those results of the parent object that you have retrieved.

The obvious performance consideration is that there is not a single round trip to the database (MongoDB instance) in order to retrieve all the information. There is always more than one.
For example, under the hood populate function looks something like this:

const users = db.users.findOne({ "_id": ObjectId("5392fea00ff066b7d533a765") });
users.items = db.items.find({ "_id": { "$in": order.items } ).toArray();

you can set mongoose debug property to true and see the number of database requests yourself.

This also makes it impossible for server side caching the data.

I am mentioning all this because if by populate in your comment you mean mongoose populate then maybe would it not be better to instead make use of a resolver instead within the createGraphQLConnection<T, U> Because the database requests will be the same in both cases just that using resolvers would enable us to implement caching using DataLoader.

[EshaanAgg]

I completely agree with your distaste for the mongoose's .populate() method, as the same is just a convenient method and saves on network requests (and not database requests). The problems you are trying to highlight here are not strictly relevant to this pagination query but rather the complete architecture of the backend.

The problem of over-reliance on the .populate method is one where most of the resolvers just populate all the fields before returning them, so convenience, they just have to write one root-level resolver and avoid writing field-level resolvers. The same can be seen from how few of the models actually have their corresponding folders in the resolvers directory! This can be another parent issue, in which we start migrating from root-level resolvers to more field-level resolvers to mitigate this dependence on .populate. On a cursory glance of the API, I can already find many such instances, and they all should be doable within a span of 2-3 weeks if other contributors are interested!

The Schema design is hacky, but it is well-suited for our current mongoose implementations. To enable server-side caching, we would probably need to move to an SQL database (not very sure about this as I haven't read much about DataLoader, @kb-0311 would be able to guide better).

[EshaanAgg]

Also, the createGraphqlConnection method is by intension, designed to be flexible and would only populate the fields you specify. So if the database queries and the resolvers are efficiency implemented, the current implementation has ZERO reliance on the .populate method. The populateField argument is currently just provided to support how all the existing resolvers work on the API side.

[kb-0311]

@EshaanAgg Yes, you are right this is related to the existing backend architecture as a whole and I just wanted to mention it to let you know about that so that you can handle that, which by your createGraphqlConnection comment I think you did.

Yeah, don't worry about that part. I had mentioned in my GSoC proposal itself on how we go about making the change and then how we get rid of N+1 database requests problem in nested relational resolver in the standard graphql pattern where the resolver name is the field name.If you do want to read about it I can send you my proposal. Also, I had asked Peter about going about making implementing large changes for ideas in this year's GSoC or Outreachy before the coding period but he said it could cause conflicts with evaluations and stuff so don't want to take that risk.

Basically, for using DataLoader caching I will HAVE to get rid of that kind of anti-pattern nature of graphql that currently exists.

It's important to note that dataloaders are not just an interface for your data models. While dataloaders are touted as a "simplified and consistent API over various remote data sources" -- their main benefit when coupled with GraphQL comes from being able to implement caching and batching within the context of a single request. This sort of functionality is important in APIs that deal with potentially redundant data (think about querying users and each user's friends -- there's a huge chance of refetching the same user multiple times). DataLoader makes sure that these kinds of queries are optimized. This is what populate function in the root resolver approach misses also.

[EshaanAgg]

Makes sense @kb-0311. The whole process of migration and upgrading the codebase to follow better practices would surely be a tedious yet rewarding one. It is imperative that we ensure all the changes being made now to the codebase align with the same, and thus greatly appreciate your concern!

[kb-0311]

@xoldyckk @EshaanAgg I think I should explain this comment you linked for Error Handling in more detail using graphql schema within the talawa-api. Please do read this approach

Let us look at the type definitions here already present in talawa-api.

  input UserInput {
    firstName: String!
    lastName: String!
    email: EmailAddress!
    password: String!
    appLanguageCode: String
    organizationUserBelongsToId: ID
  }

  type AuthData {
    user: User!
    accessToken: String!
    refreshToken: String!
    androidFirebaseOptions: AndroidFirebaseOptions!
    iosFirebaseOptions: IOSFirebaseOptions!
  }
 // Here I went ahead and made the EmailAddress Scalar nullable the rest is the same, this is important for use later
  type User {
    tokenVersion: Int!
    _id: ID!
    firstName: String!
    lastName: String!
    email: EmailAddress
    userType: String,
    appLanguageCode: String!
    createdOrganizations: [Organization]
    joinedOrganizations: [Organization]
    createdEvents: [Event]
    registeredEvents: [Event]
    eventAdmin: [Event]
    adminFor: [Organization]
    membershipRequests: [MembershipRequest]
    organizationsBlockedBy: [Organization]
    image: String
    organizationUserBelongsTo: Organization
    pluginCreationAllowed: Boolean
    adminApproved: Boolean
    createdAt: DateTime
    tagsAssignedWith(
      after: String
      before: String
      first: PositiveInt
      last: PositiveInt
      organizationId: ID
    ): UserTagsConnection
  }

// now begins the Error unions types, we can consume many error types in this
union SignUpError = EmailTaken | PasswordTooShort | UserError

type EmailTaken implements UserError {
  message: String!
  path: String!
  suggestion: String!
}

type PasswordTooShort implements UserError {
  message: String!
  path: String!
  minimumLength: Int!
}

interface UserError {
  message: String!
  path: String!
}
// Here is the return type of signup mutation  notice how the signUpData is nullable here, well that is optional.

type SignUpResult {
    signUpData : AuthData ,
    signUpErrors : [SignUpError!]
}

type Mutation {
   signUp(data: UserInput!, file: String): SignUpResult!
}

Now let us look at the root resolver part for the signUp mutation in terms of Pseudo Code, In this example itself I am taking care of all three use cases which were the main problem of my stage 5 approach->

1. Sending Multiple Errors when needed.
2. If in case of a particular error, we only want to send that error only and nothing else basically allowing atomicity of sending error if and only if the business logic for the error forces it to be that way.
3. Sending partial data not related along with the errors. For fields affected by errors send null for that field only in the data BUT send its corresponding error in the error array with a verbose Error and resolving other fields unrelated to the Error.
4. If for certain Error cases we want to return nothing as data at all we can do so

const resolvers = {
  Mutation: {
    signUp: async (parent, args, context) => {
            //Ofc I am not showing all the details of the resolver but just the general approach of how this would work.
           userObj = {} ,
           signUpErrors = []
           If(CHECK DUPLICATION OF EMAIL ) {
                 SET THE EMAIL FIELD OF  USEROBJ TO BE RETURNED AS NULL ; 
                 signUpErrors.push({
                      __typename: "EmailTaken" ,
                        message: "Email is already taken"
                        path: "UserInput.email"
                        suggestion: `Try to provide a unique mail or make sure you have not created an account 
                        already`
                 })
           }

           If (CHECK args.PASSWORD LENGTH) {
                   userErrors.push({
                      __typename: "EmailTaken" ,
                        message: "Email is already taken"
                        path: "UserInput.email"
                   })
           }
          // Approach Allowing Atomicity in sending Errors by returning from the resolver in certain Errors 
          If (CERTAIN CHECK WHERE WE WOULD NEED TO RETURN ONLY THAT ERROR IN THE 
                signUpErrors ARRAY AND NOT SEND ANY signUpData) {
                return {
                     signUpData: null,
                     signUpErrors: [{
                          __typename:"UserError" ,
                          message: "message" , 
                          path: "some path or somthing, idk"
                     }]
                 }
          }
          // Here we will be returning multiple errors in the form on an array of signUpErrors
          if (signUpErrors) {
               return  {
                   signUpData: {
                         user : userObj,
                          .... other AuthData fields
                   } , 
                   signUpErrors

               }
          }
           EVERYTHING IS OKAY, CREATE THE USER IN DB,
           return  {
                   signUpData: {
                         user : CreatedUserObj,
                          .... other AuthData fields
                   } , 
                   signUpErrors

               }


        
     }
  }
}

Now lets look at the client side part ,

mutation {
  createUser(input: {}) {
    signUpData { ... some fields }
    signUpErrors {
      # Specific cases
      ... on UserNameTaken {
        message
        path
        suggestion
      }
      # Interface contract
      ... on UserError {
        message
        path
      }
    }
  }
}

Please do tell me what do you two think about this approach, it is based on this link
https://productionreadygraphql.com/2020-08-01-guide-to-graphql-errors

[palisadoes]

My general question is, how can we use other libraries to do a majority of the work we require to easily create the customization we need?

[kb-0311]

@kb-0311 Graphql fields that you retrieve in the same resolver where you return them are applicable for stage 6-a approach. Fields that require a seperate resolver are not. For example, how will your return errors for a nested graphql connection in the parent's errors field? For a field like email it makes sense because you're resolving email in the same resolver, and therefore errors related to it can be appended to the errors list. Same isn't true for nested resolvers. You'll still need the data/errors schema for nested resolvers. With stage 5 you return data/errors for each field that could have complex logic associated to it for different kinds of clients.

Whether you use stage 5 or 6a, you can't escape implementing data/errors types for fields that have complex access control logic, if you want to communicate errors in schema.

@xoldyckk
This is just an arbitrary example

type postConnectionResult { 
    postConnectionData: PostConnectionData ,
    postConnectionErrors :[PostConnectionError!]
}

I was thinking in those cases where we would need to send either data OR error and not both we can make the nested query like postConnection return the postConnectionData as null while sending the error array with only length one. Thus we only send errors in the nested post connection field within something like the userData or OrgData field but also do we not disturb anything with the parent of the nested resolver.

          // Approach Allowing Atomicity in sending Errors by returning from the resolver in certain Errors 
          If (CERTAIN CHECK WHERE WE WOULD NEED TO RETURN ONLY THAT ERROR IN THE 
                postCOnnectionError ARRAY AND NOT SEND ANY postConnectionData) {
                return {
                     postConnectionData:null ,
                     postCOnnectionErrors: [{
                          __typename:"SomeRandomError" ,
                          message: "message" , 
                          path: "some path or somthing, idk"
                     }]
                 }
          }

Like this?

[kb-0311]

@xoldyckk So this way if we fetch postConnection field (which is also a resolver) within some parent
we will actually be returning its errors and data (could be error OR data depending on the logic) within the post connections' own graph in a nested complex query.

[xoldd]

@kb-0311 I just wanted to make sure you're aware of this. Yes, what you're showing is how it will have to be implemented for nested resolvers like connections.

You know much of the complexity can be solved by just throwing errors in resolvers, just that they won't be type safe, and on the client side it'll take a bit more logic to extract out errors. Ultimately it's up to you, and how you manage it during gsoc.

[kb-0311]

@xoldyckk Yes, I will also be updating the Error Handling doc with this approach instead in case of each errors, like you guided in the comment over there with point wise explanations of practical error cases.

Yeah true, the client side will take more logic for extracting errors including nested errors but on the bright side, Since there are graph level and field-level errors as an array the components can have multiple UI methods for relaying errors graph-wise if they do decide to do them.

But considering how many different approaches exist I think this 6a one does make sure that most use cases are covered albeit at the cost of client-side complexity.