Skip to content
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

Support SSRC conditions #2487

Merged
merged 47 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
81c90c3
Get And condition passing
erikeldridge Feb 29, 2024
fc7deeb
Support and.or condition
erikeldridge Feb 29, 2024
bd12f0d
Support true condition
erikeldridge Feb 29, 2024
43e479e
Support false condition, and fix tests
erikeldridge Feb 29, 2024
64c71d7
Use or.and, not the other way around
erikeldridge Feb 29, 2024
dd3af1d
Integrate conditional values into evaluate method
erikeldridge Mar 5, 2024
14f5339
Test handling for multiple conditions
erikeldridge Mar 5, 2024
1754fb5
Clean up logs
erikeldridge Mar 5, 2024
0283dbf
Extract condition evaluation to class for testing
erikeldridge Mar 5, 2024
264c75f
Namespace condition names
erikeldridge Mar 5, 2024
fcbcd6d
Iterate over ordered condition list
erikeldridge Mar 5, 2024
ef07c33
Test condition ordering
erikeldridge Mar 5, 2024
9179a0f
Differentiate named conditions
erikeldridge Mar 5, 2024
816f38a
Document condition types
erikeldridge Mar 5, 2024
307ec9d
Generalize condition eval test and fix styling
erikeldridge Mar 5, 2024
f87e605
Replace log statement with todo
erikeldridge Mar 5, 2024
4599ec5
Implement evaluate percent condition for RC server-side
trekforever Mar 6, 2024
559d8aa
Apply lint fixes
trekforever Mar 6, 2024
31d6c9a
Add context param to evaluate method
erikeldridge Mar 7, 2024
4e6a1c9
Add tests for percent condition eval
trekforever Mar 8, 2024
6e86cba
Update evaluator tests to use context
erikeldridge Mar 8, 2024
b669472
Increase threshold to +/- 500 for percent condition eval tests
trekforever Mar 8, 2024
687daa3
Clean up percentCondition tests a bit and add note on the tolerance used
trekforever Mar 8, 2024
12101f1
Apply suggestions from code review
erikeldridge Mar 9, 2024
89efaa7
Update copyright date and remove stray log statement
erikeldridge Mar 9, 2024
d161bd6
Mock farmhash in tests
erikeldridge Mar 11, 2024
9e0bc5d
Add Math.abs for farmhash - to be consistent with the internal implem…
trekforever Mar 12, 2024
bdc14d3
Regenerate package-lock to fix Node 14 CI error re busboy
erikeldridge Mar 13, 2024
0a3408f
Fix lint errors
erikeldridge Mar 13, 2024
347f07b
Rename "id" to "randomizationId" per discussion
erikeldridge Mar 14, 2024
6e245e2
Extract API
erikeldridge Mar 14, 2024
f7dbf1d
Only return false in cases of uknown template evaluation
erikeldridge Mar 14, 2024
0cab5d1
Remove product prefix from type names
erikeldridge Mar 14, 2024
53c2145
Remove product prefix from exported types
erikeldridge Mar 15, 2024
ee675fb
Remove unused "expression" field from server condition
erikeldridge Mar 15, 2024
c9701eb
Extract API
erikeldridge Mar 15, 2024
2f2fa2f
Merge branch 'ssrc-prefix' into ssrc-percent
erikeldridge Mar 15, 2024
fa21a91
Remove prefix from impl classes, for consistency
erikeldridge Mar 15, 2024
caddabe
Merge branch 'ssrc-prefix' into ssrc-percent
erikeldridge Mar 15, 2024
530ae21
Remove prefix from new internal classes
erikeldridge Mar 15, 2024
97876df
Remove "server" prefix
erikeldridge Mar 19, 2024
689c6aa
Remove prefix from NamedCondition
erikeldridge Mar 19, 2024
9c79eea
Merge branch 'ssrc-prefix' into ssrc-percent
erikeldridge Mar 19, 2024
0202ac7
Rename "or" and "and" fields to match API
erikeldridge Mar 20, 2024
14fdd8e
Rename "operator" field to "percentOperator" to match API
erikeldridge Mar 20, 2024
117eae9
Extract API after "and" and "or" rename
erikeldridge Mar 20, 2024
e99c489
Merge remote-tracking branch 'gh-public/ssrc' into ssrc-percent
erikeldridge Mar 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11,714 changes: 7,364 additions & 4,350 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@
"@firebase/database-compat": "^1.0.2",
"@firebase/database-types": "^1.0.0",
"@types/node": "^20.10.3",
"farmhash": "^3.3.0",
"jsonwebtoken": "^9.0.0",
"jwks-rsa": "^3.0.1",
"node-forge": "^1.3.1",
Expand Down
84 changes: 78 additions & 6 deletions src/remote-config/remote-config-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ export interface RemoteConfigCondition {

/**
* Represents a Remote Config condition in the dataplane.
* A condition targets a specific group of users. A list of these conditions make up
* part of a Remote Config template.
* A condition targets a specific group of users. A list of these conditions
* make up part of a Remote Config template.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*/
export interface RemoteConfigServerCondition {
export interface RemoteConfigServerNamedCondition {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lahirumaramba I see some interfaces with the RemoteConfig prefix and others without. Is there a guideline for what needs the prefix?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't recall a reason. Looking at this proposal (go/admin-sdk-remote-config) from 2020 it looks like the types that were added later are missing the prefix.... maybe we weren't strict about the prefix in follow up proposals?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just reviewing a change for the REST API and I think what happened is the TS API naming biased toward the REST API. The REST API has nested protos, but TS interfaces can't be nested. The parent protos had more qualified names than the child protos. When we created interfaces to match the protos, we retained the existing names.

I think the guideline we discussed against product prefixes still holds and will help the TS API be more consistent.


/**
* A non-empty and unique name of this condition.
Expand All @@ -72,7 +72,74 @@ export interface RemoteConfigServerCondition {
* {@link https://firebase.google.com/docs/remote-config/condition-reference | condition expressions}
* for the expected syntax of this field.
*/
expression: string;
condition: RemoteConfigServerCondition;
}

/**
* Represents a tree of conditions.
*/
export interface RemoteConfigServerCondition {
and?: RemoteConfigServerAndCondition;
or?: RemoteConfigServerOrCondition;
true?: RemoteConfigServerTrueCondition;
false?: RemoteConfigServerFalseCondition;
percent?: RemoteConfigServerPercentCondition;
}

/**
* Represents a collection of conditions that evaluate to true if all are true.
*/
export interface RemoteConfigServerAndCondition {
conditions?: Array<RemoteConfigServerCondition>;
}

/**
* Represents a collection of conditions that evaluate to true if any are true.
*/
export interface RemoteConfigServerOrCondition {
conditions?: Array<RemoteConfigServerCondition>;
}

/**
* Represents a condition that always evaluates to true.
*/
export interface RemoteConfigServerTrueCondition {
}

/**
* Represents a condition that always evaluates to false.
*/
export interface RemoteConfigServerFalseCondition {
}

/**
* Defines supported operators for percent conditions.
*/
export enum PercentConditionOperator {
UNKNOWN = 'UNKNOWN',
LESS_OR_EQUAL = 'LESS_OR_EQUAL',
GREATER_THAN = 'GREATER_THAN',
BETWEEN = 'BETWEEN'
}

/**
* Represents the limit of percentiles to target in micro-percents. The value
* must be in the range [0 and 100000000]
*/
export interface MicroPercentRange {
microPercentLowerBound?: number;
microPercentUpperBound?: number;
}

/**
* Represents a condition that compares the instance pseudo-random percentile to
* a given limit.
*/
export interface RemoteConfigServerPercentCondition {
operator?: PercentConditionOperator;
microPercent?: number;
seed?: string;
microPercentRange?: MicroPercentRange;
}

/**
Expand Down Expand Up @@ -195,7 +262,7 @@ export interface RemoteConfigServerTemplateData {
/**
* A list of conditions in descending order by priority.
*/
conditions: RemoteConfigServerCondition[];
conditions: RemoteConfigServerNamedCondition[];

/**
* Map of parameter keys to their optional default values and optional conditional values.
Expand Down Expand Up @@ -252,7 +319,7 @@ export interface RemoteConfigServerTemplate {
/**
* Evaluates the current template to produce a {@link RemoteConfigServerConfig}.
*/
evaluate(): RemoteConfigServerConfig;
evaluate(context?: RemoteConfigServerContext): RemoteConfigServerConfig;

/**
* Fetches and caches the current active version of the
Expand All @@ -261,6 +328,11 @@ export interface RemoteConfigServerTemplate {
load(): Promise<void>;
}

/**
* Represents template evaluation input signals.
*/
export type RemoteConfigServerContext = { [key: string]: string };

/**
* Interface representing a Remote Config user.
*/
Expand Down
173 changes: 173 additions & 0 deletions src/remote-config/remote-config-condition-evaluator-internal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/*!
* Copyright 2020 Google Inc.
erikeldridge marked this conversation as resolved.
Show resolved Hide resolved
*
* 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.
*/

'use strict';

import { isNumber } from 'lodash';
import {
RemoteConfigServerAndCondition,
RemoteConfigServerCondition,
RemoteConfigServerContext,
RemoteConfigServerNamedCondition,
RemoteConfigServerOrCondition,
RemoteConfigServerPercentCondition,
PercentConditionOperator
} from './remote-config-api';
import * as farmhash from 'farmhash';
import { FirebaseRemoteConfigError } from './remote-config-api-client-internal';

/**
* Encapsulates condition evaluation logic to simplify organization and
* facilitate testing.
*
* @internal
*/
export class RemoteConfigConditionEvaluator {
private static MAX_CONDITION_RECURSION_DEPTH = 10;

public evaluateConditions(
namedConditions: RemoteConfigServerNamedCondition[],
context: RemoteConfigServerContext): Map<string, boolean> {
// The order of the conditions is significant.
// A JS Map preserves the order of insertion ("Iteration happens in insertion order"
// - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map#description).
const evaluatedConditions = new Map();

for (const namedCondition of namedConditions) {
evaluatedConditions.set(
namedCondition.name,
this.evaluateCondition(namedCondition.condition, context));
}

return evaluatedConditions;
}

private evaluateCondition(
condition: RemoteConfigServerCondition,
context: RemoteConfigServerContext,
nestingLevel: number = 0): boolean {
if (nestingLevel >= RemoteConfigConditionEvaluator.MAX_CONDITION_RECURSION_DEPTH) {
throw new Error('Evaluating condition to false because it exceeded maximum depth ' +
RemoteConfigConditionEvaluator.MAX_CONDITION_RECURSION_DEPTH);
return false;
}
if (condition.or) {
return this.evaluateOrCondition(condition.or, context, nestingLevel + 1)
}
if (condition.and) {
return this.evaluateAndCondition(condition.and, context, nestingLevel + 1)
}
if (condition.true) {
return true;
}
if (condition.false) {
return false;
}
if (condition.percent) {
return this.evaluatePercentCondition(condition.percent, context);
}
console.log(`Evaluating unknown condition ${JSON.stringify(condition)} to false.`);
return false;
}

private evaluateOrCondition(
orCondition: RemoteConfigServerOrCondition,
context: RemoteConfigServerContext,
nestingLevel: number): boolean {

const subConditions = orCondition.conditions || [];

for (const subCondition of subConditions) {
// Recursive call.
const result = this.evaluateCondition(
subCondition, context, nestingLevel + 1);

// Short-circuit the evaluation result for true.
if (result) {
return result;
}
}
return false;
}

private evaluateAndCondition(
andCondition: RemoteConfigServerAndCondition,
context: RemoteConfigServerContext,
nestingLevel: number): boolean {

const subConditions = andCondition.conditions || [];

for (const subCondition of subConditions) {
// Recursive call.
const result = this.evaluateCondition(
subCondition, context, nestingLevel + 1);

// Short-circuit the evaluation result for false.
if (!result) {
return result;
}
}
return true;
}

private evaluatePercentCondition(
percentCondition: RemoteConfigServerPercentCondition,
context: RemoteConfigServerContext
): boolean {
const { seed, operator, microPercent, microPercentRange } = percentCondition;

if (!operator) {
throw new FirebaseRemoteConfigError('failed-precondition', 'invalid operator in remote config server condition');
}

if (!context.id) {
throw new FirebaseRemoteConfigError('failed-precondition',
'context argument is missing an "id" field.');
}

const seedPrefix = seed && seed.length > 0 ? `${seed}.` : '';
const stringToHash = `${seedPrefix}${context.id}`;
const hash64 = parseFloat(farmhash.fingerprint64(stringToHash));
const instanceMicroPercentile = hash64 % (100 * 1_000_000);

switch (operator) {
case PercentConditionOperator.LESS_OR_EQUAL:
if (isNumber(microPercent)) {
return instanceMicroPercentile <= microPercent;
}
break;
case PercentConditionOperator.GREATER_THAN:
if (isNumber(microPercent)) {
return instanceMicroPercentile > microPercent;
}
break;
case PercentConditionOperator.BETWEEN:
if (microPercentRange && isNumber(microPercentRange.microPercentLowerBound)
&& isNumber(microPercentRange.microPercentUpperBound)) {
return instanceMicroPercentile > microPercentRange.microPercentLowerBound
&& instanceMicroPercentile <= microPercentRange.microPercentUpperBound;
}
break;
case PercentConditionOperator.UNKNOWN:
default:
break;
}

throw new FirebaseRemoteConfigError(
'failed-precondition',
'invalid operator in remote config server condition');
}
}
Loading
Loading