-
Notifications
You must be signed in to change notification settings - Fork 390
Adds support for exporting a serialized config response from the RC server side SDK. #2829
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
Changes from all commits
13c1b1d
62bf436
812f374
eaf78e2
e1dff12
805dec8
e67c529
8d66a91
6b60fac
112ddfb
03dbccc
20082ef
3d750bf
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 |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
*/ | ||
|
||
import { App } from '../app'; | ||
import * as utils from '../utils/index'; | ||
import * as validator from '../utils/validator'; | ||
import { FirebaseRemoteConfigError, RemoteConfigApiClient } from './remote-config-api-client-internal'; | ||
import { ConditionEvaluator } from './condition-evaluator-internal'; | ||
|
@@ -41,6 +42,7 @@ import { | |
GetServerTemplateOptions, | ||
InitServerTemplateOptions, | ||
ServerTemplateDataType, | ||
FetchResponseData, | ||
} from './remote-config-api'; | ||
|
||
/** | ||
|
@@ -298,7 +300,7 @@ class RemoteConfigTemplateImpl implements RemoteConfigTemplate { | |
*/ | ||
class ServerTemplateImpl implements ServerTemplate { | ||
private cache: ServerTemplateData; | ||
private stringifiedDefaultConfig: {[key: string]: string} = {}; | ||
private stringifiedDefaultConfig: { [key: string]: string } = {}; | ||
|
||
constructor( | ||
private readonly apiClient: RemoteConfigApiClient, | ||
|
@@ -425,7 +427,7 @@ class ServerTemplateImpl implements ServerTemplate { | |
class ServerConfigImpl implements ServerConfig { | ||
constructor( | ||
private readonly configValues: { [key: string]: Value }, | ||
){} | ||
) { } | ||
getBoolean(key: string): boolean { | ||
return this.getValue(key).asBoolean(); | ||
} | ||
|
@@ -438,6 +440,9 @@ class ServerConfigImpl implements ServerConfig { | |
getValue(key: string): Value { | ||
return this.configValues[key] || new ValueImpl('static'); | ||
} | ||
getAll(): { [key: string]: Value } { | ||
return { ...this.configValues }; | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -613,3 +618,62 @@ class VersionImpl implements Version { | |
return validator.isNonEmptyString(timestamp) && (new Date(timestamp)).getTime() > 0; | ||
} | ||
} | ||
|
||
const HTTP_NOT_MODIFIED = 304; | ||
const HTTP_OK = 200; | ||
|
||
/** | ||
* Represents a fetch response that can be used to interact with RC's client SDK. | ||
*/ | ||
export class RemoteConfigFetchResponse { | ||
private response: FetchResponseData; | ||
|
||
/** | ||
* @param app - The app for this RemoteConfig service. | ||
* @param serverConfig - The server config for which to generate a fetch response. | ||
* @param requestEtag - A request eTag with which to compare the current response. | ||
*/ | ||
constructor(app: App, serverConfig: ServerConfig, requestEtag?: string) { | ||
const config: { [key: string]: string } = {}; | ||
for (const [param, value] of Object.entries(serverConfig.getAll())) { | ||
config[param] = value.asString(); | ||
} | ||
|
||
const currentEtag = this.processEtag(config, app); | ||
|
||
if (currentEtag === requestEtag) { | ||
this.response = { | ||
status: HTTP_NOT_MODIFIED, | ||
eTag: currentEtag, | ||
}; | ||
} else { | ||
this.response = { | ||
status: HTTP_OK, | ||
eTag: currentEtag, | ||
config, | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* @returns JSON representation of the fetch response that can be consumed | ||
* by the RC client SDK. | ||
*/ | ||
Comment on lines
+658
to
+661
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. Probably needs to be annotated with 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. Hmm, I don't actually know - none of the other public methods are annotated with an |
||
public toJSON(): FetchResponseData { | ||
return this.response; | ||
} | ||
|
||
private processEtag(config: { [key: string]: string }, app: App): string { | ||
const configJson = JSON.stringify(config); | ||
let hash = 0; | ||
// Mimics Java's `String.hashCode()` which is used in RC's servers. | ||
for (let i = 0; i < configJson.length; i++) { | ||
const char = configJson.charCodeAt(i); | ||
hash = (hash << 5) - hash + char; | ||
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. This is quite cool. Should we add a comment describing the equation and also that we're trying to replicate the 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. Comment added |
||
hash |= 0; | ||
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. What is the significance of this operation? 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. Added a comment per below comment |
||
} | ||
const projectId = utils.getExplicitProjectId(app); | ||
const parts = ['etag', projectId, 'firebase-server', 'fetch', hash]; | ||
return parts.filter(a => !!a).join('-'); | ||
} | ||
} |
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.
What's the request in this context? Do we expect the client to pass an etag to the customer server which can be used to avoid a config pushdown?
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.
That's correct - this would come in from a client request. it should be the etag of the most recent config that the client has. Since our internal servers do this, it makes sense to allow customer servers to do it as well