-
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
Changes from 24 commits
bf9c81f
778f20e
b6ce5d9
99ef146
4b8ab09
60cbaed
d066264
fa5d14b
591ce30
fa56366
850781c
09ee75d
115b40a
7f4a02a
92030db
a42e8c6
e4bf66c
6faca17
54a0889
f2ad045
b532c8d
2e5700a
31151be
b3eda27
3f97583
09233b5
f009f7b
c9a7153
b5723a0
afc8326
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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: string | undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Dan, just wondering is this "restParameter" expecting a boolean type from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the type to |
||
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: string | undefined, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe here change the type to Fallback to be consistent too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
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: string | undefined; | ||
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); | ||
}); | ||
}); | ||
}); | ||
} | ||
); | ||
}); | ||
}); |
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.