Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/mi 19 bigcommerce token and customer services #2

Open
wants to merge 8 commits into
base: version/bigcommerce-1.1.0
Choose a base branch
from

Conversation

tvhees
Copy link
Contributor

@tvhees tvhees commented Oct 29, 2024

Provide services for the following operations commonly used in the mesh:

  • Get the customer impersonation token, from the cache if possible
  • Generate a JWT for a logged in customer, and extract their customer id from such a JWT

I have also:

  • deprecated existing methods for these operations
  • used the token service for automatically injecting the customer impersonation token in the graphql client
  • modified two resolvers to use the new services as examples

@tvhees
Copy link
Contributor Author

tvhees commented Oct 30, 2024

@brettcutt-aligent I see that you've previously implemented this as a Request class in client work, which I think makes sense for at least the customerId part of this. I'm interested in where you think the service boundaries should be here (general tokens, per-request information, customer information etc.) and very open to re-architecting this

@tvhees tvhees requested a review from ryanrixxh October 30, 2024 01:34
@brettcutt-aligent
Copy link
Contributor

@brettcutt-aligent I see that you've previously implemented this as a Request class in client work, which I think makes sense for at least the customerId part of this. I'm interested in where you think the service boundaries should be here (general tokens, per-request information, customer information etc.) and very open to re-architecting this

@tvhees What you have in this PR looks good and follows along with the discussion of last weeks meeting. Ideally I would rather have a service which handles all the required request headers in one spot but couldn't be sure of the best approach to allow certain requests even if authentication fails e.g. storeConfigs`.

import { sign } from 'jsonwebtoken';

// The default "customer_impersonation_token" query TTL should one not be stored in "CACHE_ITEMS_TTL"
const QUERY_DEFAULT_TTL = CACHE_ITEMS_TTL?.[CACHE_KEY__CUSTOMER_IMPERSONATION_TOKEN] || 86400000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to set a fallback value if there's already one set in constants.ts?``

const QUERY_DEFAULT_TTL = CACHE_ITEMS_TTL?.[CACHE_KEY__CUSTOMER_IMPERSONATION_TOKEN] || 86400000;

// We time we extend the tokens' Time To Live (TTL) beyond the duration it's stored in the cache
const QUERY_TTL_BUFFER_IN_MILLISECONDS = 10 * 60 * 1000; // minutes x seconds/min x ms/second
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this live in constants.ts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR but it would be great if we could extract the xray logic into sub functions. I find the main caching logic is hard to read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants