-
Notifications
You must be signed in to change notification settings - Fork 5
feat(core): Implement ApiSession and ApiSessionUtils with session management #434
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
Merged
Merged
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
0825eb2
feat: Implement ApiSession and ApiSessionUtils with session managemen…
richie-ni b35328e
Merge branch 'main' into users/richie/create-api-session-key
richie-ni 8cb87c6
feat(api-session): Enhance ApiSessionUtils with session caching and i…
richie-ni 59aa1d2
fix(api-session): Update session key references to use 'session' inst…
richie-ni 47d2cc5
fix(api-session): Update mock session endpoint in tests to use 'http:…
richie-ni 7ac0a9b
fix(api-session): Improve error handling in createApiSession method w…
richie-ni 4b336c4
docs: Update README and example.yaml for clearer API and UI ingress i…
richie-ni a68cc73
fix(example.yaml): Remove trailing space in SystemLink Products datas…
richie-ni abc0587
test(DataSourceBase): Add unit tests for GET and POST methods with AP…
richie-ni 518c547
fix(README.md): Adjust table formatting for clarity in contribution g…
richie-ni 6240ce4
refactor(api-session): Convert session cache to static property and u…
richie-ni ce60ea7
fix(ApiSessionUtils): Simplify session creation logic and update erro…
richie-ni ceba513
fix(README.md): Update instructions to navigate to the Application Ta…
richie-ni 485bca1
fix(api-session): Rename 'session' to 'sessionKey' for consistency in…
richie-ni 568cc4f
fix(comments): Restructure ApiSessionUtils and update session handlin…
richie-ni 1a2f21f
Added test cases
richie-ni b0028e4
fix(test): Update POST request parameters to include body and options…
richie-ni e9530ad
refactor(api-session): Improve ApiSessionUtils constructor and update…
richie-ni f4d4eb9
fix(api-session): Update createApiSession return type to NonNullable …
richie-ni 50c4009
test(DataSourceBase): Add POST request test for API ingress with no o…
richie-ni 0dbf679
Merge branch 'main' into users/richie/create-api-session-key
richie-ni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import { DataSourceInstanceSettings } from "@grafana/data"; | ||
| import { of } from "rxjs"; | ||
|
|
||
| type MockedBackendSrv = jest.Mocked<{ | ||
| get: jest.Mock; | ||
| post: jest.Mock; | ||
| fetch: jest.Mock; | ||
| }>; | ||
|
|
||
| describe('DataSourceBase', () => { | ||
| let instanceSettings: DataSourceInstanceSettings; | ||
| let backendSrv: MockedBackendSrv; | ||
| let DataSourceBaseStub: any; | ||
| let dataSource: any; | ||
|
|
||
| const mockApiSession = { | ||
| endpoint: 'http://api-ingress.com', | ||
| sessionKey: { secret: 'api-key-secret' } | ||
| }; | ||
| const mockApiSessionUtils = { | ||
| createApiSession: jest.fn().mockResolvedValue(mockApiSession) | ||
| }; | ||
|
|
||
| beforeAll(async () => { | ||
| jest.resetModules(); | ||
|
|
||
| const { DataSourceBase } = await import('./DataSourceBase'); | ||
| DataSourceBaseStub = DataSourceBase; | ||
| backendSrv = { | ||
| get: jest.fn(), | ||
| post: jest.fn(), | ||
| fetch: jest.fn().mockReturnValue(of({ data: 'test' })) | ||
richie-ni marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }; | ||
| instanceSettings = { | ||
| url: 'http://api-example.com', | ||
| } as DataSourceInstanceSettings; | ||
|
|
||
| dataSource = new DataSourceBaseStub( | ||
richie-ni marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| instanceSettings, | ||
| backendSrv, | ||
| {} | ||
| ); | ||
| }); | ||
|
|
||
| describe('get', () => { | ||
| it('should send GET request with correct parameters when useApiIngress is not set', async () => { | ||
| const response = await dataSource.get('/test-endpoint', { param1: 'value1' }); | ||
|
|
||
| expect(backendSrv.fetch).toHaveBeenCalledWith({ | ||
| method: 'GET', | ||
| url: '/test-endpoint', | ||
| params: { param1: 'value1' }, | ||
| }); | ||
| expect(response).toEqual('test'); | ||
| }); | ||
|
|
||
| it('should send GET request with API ingress when useApiIngress is true', async () => { | ||
| dataSource.apiSession = mockApiSessionUtils; | ||
richie-ni marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| const response = await dataSource.get('/test-endpoint', { param1: 'value1' }, true); | ||
|
|
||
|
|
||
richie-ni marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| expect(mockApiSessionUtils.createApiSession).toHaveBeenCalled(); | ||
| expect(backendSrv.fetch).toHaveBeenCalledWith({ | ||
|
|
||
richie-ni marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| method: 'GET', | ||
| url: 'http://api-ingress.com/test-endpoint', | ||
| params: { param1: 'value1', 'x-ni-api-key': 'api-key-secret' } | ||
| } | ||
| ); | ||
| expect(response).toEqual('test'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('post', () => { | ||
| it('should send POST request with correct parameters when useApiIngress is not set', async () => { | ||
| const response = await dataSource.post('/test-endpoint', { option1: 'optionValue' }); | ||
|
|
||
| expect(backendSrv.fetch).toHaveBeenCalledWith({ | ||
| method: 'POST', | ||
| url: '/test-endpoint', | ||
| data: { option1: 'optionValue' }, | ||
| }); | ||
| expect(response).toEqual('test'); | ||
| }); | ||
|
|
||
| it('should send POST request with API ingress when useApiIngress is true', async () => { | ||
| dataSource.apiSession = mockApiSessionUtils; | ||
|
|
||
| const response = await dataSource.post('/test-endpoint', { option1: 'optionValue' }, {}, true); | ||
|
|
||
| expect(mockApiSessionUtils.createApiSession).toHaveBeenCalled(); | ||
| expect(backendSrv.fetch).toHaveBeenCalledWith({ | ||
| method: 'POST', | ||
| url: 'http://api-ingress.com/test-endpoint', | ||
| data: { option1: 'optionValue' }, | ||
| headers: { 'x-ni-api-key': 'api-key-secret' }, | ||
| }); | ||
| expect(response).toEqual('test'); | ||
| }); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import { AppEvents, DataSourceInstanceSettings } from '@grafana/data'; | ||
| import { BackendSrv } from '@grafana/runtime'; | ||
|
|
||
| jest.mock('@grafana/runtime', () => ({ | ||
| ...jest.requireActual('@grafana/runtime'), | ||
| getAppEvents: jest.fn(), | ||
| })); | ||
| jest.mock('./utils', () => ({ | ||
| post: jest.fn(), | ||
| })); | ||
| jest.resetModules(); | ||
|
|
||
| let post, getAppEvents, ApiSessionUtils; | ||
| let mockGetAppEvents: jest.Mock; | ||
| let mockPost: jest.Mock; | ||
|
|
||
richie-ni marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| describe('ApiSessionUtils', () => { | ||
| let instanceSettings: DataSourceInstanceSettings; | ||
| let backendSrv: BackendSrv; | ||
| let appEvents: { publish: any; }; | ||
| let apiSessionUtils: any; | ||
|
|
||
| beforeEach(async () => { | ||
| // Dynamically import dependencies after mocks | ||
| ApiSessionUtils = (await import('./api-session.utils')).ApiSessionUtils; | ||
| getAppEvents = (await import('@grafana/runtime')).getAppEvents; | ||
| post = (await import('./utils')).post; | ||
richie-ni marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // Reset static cache before each test | ||
| (ApiSessionUtils as any)._sessionCache = undefined; | ||
|
|
||
| mockGetAppEvents = getAppEvents as jest.Mock; | ||
| mockPost = post as jest.Mock; | ||
|
|
||
| instanceSettings = { url: 'http://api-example.com' } as DataSourceInstanceSettings; | ||
| backendSrv = {} as BackendSrv; | ||
| appEvents = { publish: jest.fn() }; | ||
| mockGetAppEvents.mockReturnValue(appEvents); | ||
|
|
||
| apiSessionUtils = new ApiSessionUtils(instanceSettings, backendSrv); | ||
|
|
||
| mockPost.mockClear(); | ||
| appEvents.publish.mockClear(); | ||
| }); | ||
|
|
||
| describe('createApiSession', () => { | ||
| const createMockSession = (expiryOffset: number) => ({ | ||
| endpoint: 'http://test-endpoint', | ||
| sessionKey: { | ||
| expiry: new Date(Date.now() + expiryOffset).toISOString(), | ||
| secret: 'test-secret', | ||
| }, | ||
| }); | ||
|
|
||
| it('should create a new session if cache is empty', async () => { | ||
| const newSession = createMockSession(600_000); // 10 minutes expiry | ||
| mockPost.mockResolvedValue(newSession); | ||
|
|
||
| const session = await apiSessionUtils.createApiSession(); | ||
|
|
||
| expect(mockPost).toHaveBeenCalledTimes(1); | ||
| expect(session).toBe(newSession); | ||
| }); | ||
|
|
||
| it('should return a valid cached session', async () => { | ||
| const validSession = createMockSession(600_000); // 10 minutes expiry, well outside 5 min buffer | ||
| mockPost.mockResolvedValue(validSession); | ||
|
|
||
| const session1 = await apiSessionUtils.createApiSession(); | ||
| const session2 = await apiSessionUtils.createApiSession(); | ||
|
|
||
| expect(mockPost).toHaveBeenCalledTimes(1); | ||
| expect(session1).toBe(validSession); | ||
| expect(session2).toBe(validSession); | ||
| }); | ||
|
|
||
| it('should create a new session if cached session is expired', async () => { | ||
| const expiredSession = createMockSession(240_000); // 4 minutes expiry (inside 5-minute buffer) | ||
| const newSession = createMockSession(600_000); | ||
| mockPost.mockResolvedValueOnce(expiredSession) | ||
| .mockResolvedValueOnce(newSession); | ||
|
|
||
| const session1 = await apiSessionUtils.createApiSession(); | ||
| const session2 = await apiSessionUtils.createApiSession(); | ||
|
|
||
| expect(mockPost).toHaveBeenCalledTimes(2); | ||
| expect(session1).toBe(expiredSession); | ||
| expect(session2).toBe(newSession); | ||
| }); | ||
|
|
||
| it('should handle errors during session creation and publish an event', async () => { | ||
| const error = new Error('Network error'); | ||
| const errorMessage = `The query to create an API session failed. ${error?.message}.` | ||
|
|
||
| mockPost.mockRejectedValue(error); | ||
|
|
||
| await expect(apiSessionUtils.createApiSession()).rejects.toThrow( | ||
| errorMessage | ||
| ); | ||
|
|
||
| expect(appEvents.publish).toHaveBeenCalledWith({ | ||
| type: AppEvents.alertError.name, | ||
| payload: [ | ||
| 'Error creating session', | ||
| errorMessage | ||
| ], | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.