Skip to content

Conversation

ItsSudip
Copy link
Member

@ItsSudip ItsSudip commented May 26, 2025

Description

This PR refactors the OAuth v2 system by simplifying the RefreshTokenParams struct and removing the dependency on the full DestinationInfo object. Instead of passing the entire destination object, it now only passes the specific DestinationID string that's actually needed.

Key Changes:

1. Simplified RefreshTokenParams Structure (services/oauth/v2/types.go):

Removed: Destination *DestinationInfo field
Added: DestinationID string field
Improved: Field alignment and formatting for better readability

2. Updated Token Generation (router/batchrouter/asyncdestinationmanager/bing-ads/common/token.go):

  • Eliminated the creation of a DestinationInfo object in GenerateTokenV2()
  • Now directly passes DestinationID instead of creating and passing the entire destination object
  • Reduced code complexity: Removed 5 lines of unnecessary object construction

3. Streamlined HTTP Transport (services/oauth/v2/http/transport.go):

  • Updated OAuth transport to pass rts.destination.ID directly instead of the full destination object
  • Maintains the same functionality while reducing object dependencies

4. Updated Logging (services/oauth/v2/oauth.go):

  • Modified logging to use refTokenParams.DestinationID directly instead of accessing it through the destination object
  • Maintains the same logging information with cleaner access pattern

5. Comprehensive Test Updates (services/oauth/v2/oauth_test.go):

  • Updated all test cases (102 lines changed) to use the new DestinationID field
  • Improved test parameter formatting for better readability
  • All tests maintain the same coverage and functionality

Benefits:

  • Reduced Memory Footprint: No longer passing entire DestinationInfo objects when only the ID is needed
  • Cleaner API: More explicit about what data is actually required
  • Better Separation of Concerns: RefreshTokenParams now only contains the minimal data needed for token operations
  • Improved Maintainability: Fewer dependencies between components
  • Consistent Formatting: Better code alignment and readability

Impact:

This is a non-breaking refactoring that maintains all existing functionality while improving the internal architecture. The change is purely structural and doesn't affect the external behavior of the OAuth system.

Linear Ticket

https://linear.app/rudderstack/issue/INT-3721/clean-up-destinationinfo-from-refreshtokenparams

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

Copy link

codecov bot commented May 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.88%. Comparing base (382ae22) to head (cf1815c).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5897      +/-   ##
==========================================
- Coverage   76.94%   76.88%   -0.07%     
==========================================
  Files         494      494              
  Lines       68240    68235       -5     
==========================================
- Hits        52508    52460      -48     
- Misses      12863    12906      +43     
  Partials     2869     2869              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ItsSudip ItsSudip merged commit 4eb40cd into master May 27, 2025
62 checks passed
@ItsSudip ItsSudip deleted the chore/refactorRefreshTokenParams branch May 27, 2025 09:17
atzoum pushed a commit that referenced this pull request Jun 9, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.51.0-rc.2](v1.50.0...v1.51.0-rc.2)
(2025-06-09)


### Features

* integrate the transformer client with cslb for source transformations
([#5881](#5881))
([53b8156](53b8156))
* triggers deployment for staging and hosted releases
([#5950](#5950))
([c8d849e](c8d849e))


### Bug Fixes

* block internal IP access as destination
([#5888](#5888))
([a0a4b91](a0a4b91))
* collected metric named router_process_jobs_count collides with
previously collected histogram named router_process_jobs
([#5966](#5966))
([d60d224](d60d224))
* **processor:** throughput stalling due to mutex deadlock on full
connection pool
([#5919](#5919))
([b18df16](b18df16))
* under reporting of load files when using upload id
([#5889](#5889))
([4a3a11c](4a3a11c))
* under reporting of load files when using upload id
([#5889](#5889))
([#5906](#5906))
([4f300cf](4f300cf))
* use jsonrs stdlib for encoding warehouse transformations
([#5945](#5945))
([0963a17](0963a17))
* warehouse transformations geo enrichment as string
([#5907](#5907))
([4f300cf](4f300cf))
* warehouse transformations responses ordering
([#5954](#5954))
([ea1a527](ea1a527))


### Miscellaneous

* add config to disable load table stats for clickhouse
([#5932](#5932))
([55c26f2](55c26f2))
* add event delivery metrics for async destinations
([#5930](#5930))
([7e9f1b5](7e9f1b5))
* add new metric regulation_worker_deletion_status_count
([#5939](#5939))
([1aa00f9](1aa00f9))
* add test case for dynamic config pattern with numeric default value
([#5904](#5904))
([9cd7180](9cd7180))
* clean up destinationInfo from refreshTokenParams
([#5897](#5897))
([4eb40cd](4eb40cd))
* **deps:** bump github.com/apache/pulsar-client-go from
0.15.1-candidate-2 to 0.15.1
([#5895](#5895))
([4a3a11c](4a3a11c))
* fix aws sdk v2
([#5928](#5928))
([e5b0ab4](e5b0ab4))
* flakiness in upload jobs count
([#5913](#5913))
([b167104](b167104))
* proc sample store uploader
([#5917](#5917))
([2e3b796](2e3b796))
* remove unused schemas repo in warehouse router
([#5879](#5879))
([4e96935](4e96935))
* **router:** adaptive throttler decreasing throttling rate dynamically
based on current throttled rate
([#5931](#5931))
([3d3b5b3](3d3b5b3))
* **router:** use different stats for capturing process stage jobs and
requests
([#5944](#5944))
([6728b2a](6728b2a))
* use zero transport on new common reporting client
([#5959](#5959))
([d35e2cc](d35e2cc))
* warehouse transformations json diff with encoding
([#5937](#5937))
([2e3b796](2e3b796))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
atzoum pushed a commit that referenced this pull request Jun 9, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.51.0-rc.3](v1.50.0...v1.51.0-rc.3)
(2025-06-09)


### Features

* integrate the transformer client with cslb for source transformations
([#5881](#5881))
([53b8156](53b8156))
* triggers deployment for staging and hosted releases
([#5950](#5950))
([c8d849e](c8d849e))


### Bug Fixes

* block internal IP access as destination
([#5888](#5888))
([a0a4b91](a0a4b91))
* collected metric named router_process_jobs_count collides with
previously collected histogram named router_process_jobs
([#5966](#5966))
([d60d224](d60d224))
* **processor:** throughput stalling due to mutex deadlock on full
connection pool
([#5919](#5919))
([b18df16](b18df16))
* router request failing with unsupported media type error
([#5968](#5968))
([edaa10c](edaa10c))
* under reporting of load files when using upload id
([#5889](#5889))
([4a3a11c](4a3a11c))
* under reporting of load files when using upload id
([#5889](#5889))
([#5906](#5906))
([4f300cf](4f300cf))
* use jsonrs stdlib for encoding warehouse transformations
([#5945](#5945))
([0963a17](0963a17))
* warehouse transformations geo enrichment as string
([#5907](#5907))
([4f300cf](4f300cf))
* warehouse transformations responses ordering
([#5954](#5954))
([ea1a527](ea1a527))


### Miscellaneous

* add config to disable load table stats for clickhouse
([#5932](#5932))
([55c26f2](55c26f2))
* add event delivery metrics for async destinations
([#5930](#5930))
([7e9f1b5](7e9f1b5))
* add new metric regulation_worker_deletion_status_count
([#5939](#5939))
([1aa00f9](1aa00f9))
* add test case for dynamic config pattern with numeric default value
([#5904](#5904))
([9cd7180](9cd7180))
* clean up destinationInfo from refreshTokenParams
([#5897](#5897))
([4eb40cd](4eb40cd))
* **deps:** bump github.com/apache/pulsar-client-go from
0.15.1-candidate-2 to 0.15.1
([#5895](#5895))
([4a3a11c](4a3a11c))
* fix aws sdk v2
([#5928](#5928))
([e5b0ab4](e5b0ab4))
* flakiness in upload jobs count
([#5913](#5913))
([b167104](b167104))
* proc sample store uploader
([#5917](#5917))
([2e3b796](2e3b796))
* remove unused schemas repo in warehouse router
([#5879](#5879))
([4e96935](4e96935))
* **router:** adaptive throttler decreasing throttling rate dynamically
based on current throttled rate
([#5931](#5931))
([3d3b5b3](3d3b5b3))
* **router:** use different stats for capturing process stage jobs and
requests
([#5944](#5944))
([6728b2a](6728b2a))
* use zero transport on new common reporting client
([#5959](#5959))
([d35e2cc](d35e2cc))
* warehouse transformations json diff with encoding
([#5937](#5937))
([2e3b796](2e3b796))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

3 participants