tailcallhq / tailcall

High Performance GraphQL Runtime
https://tailcall.run
Apache License 2.0
1.26k stars 237 forks source link

Add support for `skipNull: true` in HTTP directive query parameters #2468

Closed amitksingh1490 closed 3 weeks ago

amitksingh1490 commented 1 month ago

Feature Request

Summary

The current GraphQL schema allows for HTTP requests to be configured via the @http directive. However, there is a need to support skipping null query parameters to avoid sending unnecessary or invalid data in requests.

After https://github.com/tailcallhq/tailcall/issues/2467 the default behaviour will not skip the null params

Example

Consider the following GraphQL type definition:

type User {
  posts: [Post]
    @http(
      path: "/posts"
      query: [{ key: "author_ids[]", value: "{{.value.authorID}}", skipNull: true }]
      batchKey: ["author_ids[]"]
    )
}

In the above example, if {{.value.authorID}} resolves to null, the author_ids[] parameter would be skipped entirely in the HTTP request.

skipNull: true when specified in query params will skip the param whole value is null the default value of this argument is false

tusharmath commented 1 month ago

/bounty 50$

algora-pbc[bot] commented 1 month ago

💎 $50 bounty • Tailcall Inc.

Steps to solve:

  1. Start working: Comment /attempt #2468 with your implementation plan
  2. Submit work: Create a pull request including /claim #2468 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

🙏 Thank you for contributing to tailcallhq/tailcall! 🧐 Checkout our guidelines before you get started. 💵 More about our bounty program.

Attempt Started (GMT+0) Solution
🔴 @bnchi Jul 19, 2024, 12:22:29 PM WIP
tusharmath commented 1 month ago

Very easy fix, checkout this PR https://github.com/tailcallhq/tailcall/pull/2470

ssddOnTop commented 1 month ago

/attempt

bnchi commented 1 month ago

/attempt #2468

Algora profile Completed bounties Tech Active attempts Options
@bnchi 3 tailcallhq bounties
TypeScript, Rust,
HTML & more
Cancel attempt
bnchi commented 1 month ago

@ssddOnTop go ahead

algora-pbc[bot] commented 1 month ago

Here are some steps and pointers to help you get started on resolving this issue:

  1. Update Directive Parsing Logic:

    • Modify the directive parsing logic to recognize and handle the skipNull option.
    • Ensure that the default value for skipNull is false.
  2. Modify Query Parameter Handling:

    • Update the logic that constructs query parameters to check the skipNull flag.
    • If skipNull is true and the parameter value is null, skip adding that parameter to the query string.
  3. Update Tests:

    • Add test cases to ensure that query parameters with skipNull: true are correctly skipped when their values are null.
    • Ensure existing tests still pass and cover scenarios where skipNull is not specified or is false.

Relevant Files and Code References

  1. Directive Parsing Logic:

    • The directive parsing logic is likely located in the core GraphQL schema processing code. Look for files related to directive handling and parsing.
  2. Query Parameter Handling:

    • The logic for constructing query parameters can be found in the HTTP request handling code. This is where you will add the check for skipNull.
  3. Tests:

    • /tests/execution/test-directives-undef-null-fields.md: This file contains tests related to handling undefined and null fields in directives. Update this file to include tests for skipNull.
    • /tests/http/config/simple.graphql: This file defines a GraphQL schema with HTTP directives. Add test cases here to ensure the new skipNull functionality works as expected.
    • /tests/execution/nullable-arg-query.md: This file contains tests for nullable arguments in queries. Ensure that the new skipNull functionality is covered here as well.

Example Code Snippet

Here is an example of how you might modify the query parameter handling logic:

function constructQueryParams(params) {
  const queryParams = [];
  for (const param of params) {
    const { key, value, skipNull } = param;
    if (skipNull && value === null) {
      continue;
    }
    queryParams.push(`${encodeURIComponent(key)}=${encodeURIComponent(value)}`);
  }
  return queryParams.join('&');
}

Potential Implications

  1. Security:

    • Ensure that skipping null parameters does not inadvertently expose sensitive information or lead to unintended behavior in the API.
  2. Stability:

    • Thoroughly test the new functionality to ensure it does not introduce regressions or unexpected behavior in existing functionality.
  3. Potential Bugs:

    • Be cautious of edge cases where the value might be undefined instead of null. Ensure that the logic correctly differentiates between the two.
tusharmath commented 1 month ago

I think skipNull is a bit to specific. Ideally, we would want to skip on various values for eg: "Skip with value is equal to something" etc. The current implementation of dispatching query params even when there are no values assigned works for most usecases.