-
Notifications
You must be signed in to change notification settings - Fork 99
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
feat: Allow the user to set fallback to rest #1203
Merged
danieljbruce
merged 30 commits into
googleapis:main
from
danieljbruce:use-http-fallback
Dec 11, 2023
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
bf9c81f
Allow the user to set fallback to rest
danieljbruce 778f20e
Merge branch 'main' into use-http-fallback
danieljbruce b6ce5d9
Putting together a test that mocks out gapic
danieljbruce 99ef146
Merge branch 'use-http-fallback' of https://github.com/danieljbruce/n…
danieljbruce 4b8ab09
This harness works properly
danieljbruce 60cbaed
Add client testing module
danieljbruce d066264
Regroup the code so it is better arranged
danieljbruce fa5d14b
Remove the datastore mock
danieljbruce 591ce30
Add comments for test helper functions. Add tests for the non rest case.
danieljbruce fa56366
Clean up source code change
danieljbruce 850781c
Add parameterized test
danieljbruce 09ee75d
Document the class for mocking datastore client
danieljbruce 115b40a
Remove only
danieljbruce 7f4a02a
Remove fallback params type
danieljbruce 92030db
Remove changes to system tests
danieljbruce a42e8c6
Add header
danieljbruce e4bf66c
Check for client existence
danieljbruce 6faca17
Change it back
danieljbruce 54a0889
Add more debugger logs
danieljbruce f2ad045
Add project load logs
danieljbruce b532c8d
console error
danieljbruce 2e5700a
Add comment for mocking out project id
danieljbruce 31151be
linting fix
danieljbruce b3eda27
Correct title of describe block
danieljbruce 3f97583
Change rest parameter in test to fallback
danieljbruce 09233b5
Create a simple system test for using rest
danieljbruce f009f7b
Should check to see if entity is undefined
danieljbruce c9a7153
Merge branch 'main' into use-http-fallback
danieljbruce b5723a0
Specify fallback type instead of string | undefin
danieljbruce afc8326
Merge branch 'use-http-fallback' of https://github.com/danieljbruce/n…
danieljbruce File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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 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,165 @@ | ||
// Copyright 2023 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
import {beforeEach, describe, it} from 'mocha'; | ||
import { | ||
Datastore, | ||
DatastoreClient, | ||
Fallback, | ||
DatastoreRequest, | ||
DatastoreOptions, | ||
} from '../../src'; | ||
import * as assert from 'assert'; | ||
import * as proxyquire from 'proxyquire'; | ||
import {Callback, CallOptions} from 'google-gax'; | ||
import * as protos from '../../protos/protos'; | ||
import * as ds from '../../src'; | ||
import * as mocha from 'mocha'; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
type Any = any; | ||
|
||
const clientName = 'DatastoreClient'; | ||
const async = require('async'); | ||
|
||
/** | ||
* This class mocks out the lookup function so that for tests in this file | ||
* the lookup function just sends data back instead of making a call to the | ||
* server. The class also saves the rest parameter in the constructor so that | ||
* it can be read later for correctness. | ||
* | ||
*/ | ||
class FakeDatastoreClient extends DatastoreClient { | ||
restParameter: Fallback; | ||
constructor(...args: any[]) { | ||
super(); | ||
this.restParameter = args[0].fallback; | ||
} | ||
lookup( | ||
request?: protos.google.datastore.v1.ILookupRequest, | ||
optionsOrCallback?: | ||
| CallOptions | ||
| Callback< | ||
protos.google.datastore.v1.ILookupResponse, | ||
protos.google.datastore.v1.ILookupRequest | null | undefined, | ||
{} | null | undefined | ||
>, | ||
callback?: Callback< | ||
protos.google.datastore.v1.ILookupResponse, | ||
protos.google.datastore.v1.ILookupRequest | null | undefined, | ||
{} | null | undefined | ||
> | ||
): Promise< | ||
[ | ||
protos.google.datastore.v1.ILookupResponse, | ||
protos.google.datastore.v1.ILookupRequest | undefined, | ||
{} | undefined, | ||
] | ||
> { | ||
if (callback) { | ||
callback(null, {}); | ||
} | ||
return new Promise((resolve, reject) => { | ||
resolve([{}, {}, {}]); | ||
}); | ||
} | ||
} | ||
|
||
describe('Client Initialization Testing', () => { | ||
describe('Request', () => { | ||
let Request: typeof ds.DatastoreRequest; | ||
let request: Any; | ||
|
||
/** | ||
* This function is called by a test to ensure that the rest parameter | ||
* gets passed to the Gapic data client's constructor | ||
* | ||
* @param {DatastoreRequest} [request] The request object whose data client | ||
* the test makes comparisons with. | ||
* @param {string | undefined} [expectedFallback] The value that the test | ||
* expects the rest parameter of the data client to be equal to. | ||
* @param {mocha.Done} [done] The done function used for indicating | ||
* that the test is complete or that there is an error in the mocha test | ||
* environment. | ||
* | ||
*/ | ||
function compareRequest( | ||
request: DatastoreRequest, | ||
expectedFallback: Fallback, | ||
done: mocha.Done | ||
) { | ||
try { | ||
const client = request.datastore.clients_.get(clientName); | ||
assert(client); | ||
assert.strictEqual(client.restParameter, expectedFallback); | ||
done(); | ||
} catch (err: unknown) { | ||
done(err); | ||
} | ||
} | ||
async.each( | ||
[ | ||
{ | ||
options: {fallback: 'rest' as Fallback}, | ||
expectedFallback: 'rest', | ||
description: 'when specifying rest as a fallback parameter', | ||
}, | ||
{ | ||
options: {}, | ||
expectedFallback: undefined, | ||
description: 'when specifying no fallback parameter', | ||
}, | ||
], | ||
(testParameters: { | ||
options: DatastoreOptions; | ||
expectedFallback: Fallback; | ||
description: string; | ||
}) => { | ||
describe(testParameters.description, () => { | ||
beforeEach(() => { | ||
Request = proxyquire('../../src/request', { | ||
'./v1': { | ||
DatastoreClient: FakeDatastoreClient, | ||
}, | ||
}).DatastoreRequest; | ||
request = new Request(); | ||
request.datastore = new Datastore(testParameters.options); | ||
// The CI environment can't fetch project id so the function that | ||
// fetches the project id needs to be mocked out. | ||
request.datastore.auth.getProjectId = ( | ||
callback: (err: any, projectId: string) => void | ||
) => { | ||
callback(null, 'some-project-id'); | ||
}; | ||
}); | ||
it('should set the rest parameter in the data client when calling prepareGaxRequest_', done => { | ||
// This request does lazy initialization of the gapic layer Datastore client. | ||
request.prepareGaxRequest_( | ||
{client: clientName, method: 'lookup'}, | ||
() => { | ||
compareRequest(request, testParameters.expectedFallback, done); | ||
} | ||
); | ||
}); | ||
it('should set the rest parameter in the data client when calling request_', done => { | ||
// This request does lazy initialization of the gapic layer Datastore client. | ||
request.request_({client: clientName, method: 'lookup'}, () => { | ||
compareRequest(request, testParameters.expectedFallback, done); | ||
}); | ||
}); | ||
}); | ||
} | ||
); | ||
}); | ||
}); |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is already commented somewhere else. Do you think we can provide an example how to initiate client library with
fallback
options?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a simple system test for this.