Represent feature flags using an enum

Replaces the previous string literal type
This commit is contained in:
Henry Mercer 2021-12-16 13:14:32 +00:00
parent 5e87034b3b
commit 6d62c245ec
12 changed files with 88 additions and 108 deletions

View file

@ -10,7 +10,7 @@ import * as apiClient from "./api-client";
import { setCodeQL } from "./codeql";
import { Config } from "./config-utils";
import { uploadDatabases } from "./database-upload";
import { createFeatureFlags, FeatureFlags } from "./feature-flags";
import { createFeatureFlags, FeatureFlag, FeatureFlags } from "./feature-flags";
import { Language } from "./languages";
import { RepositoryNwo } from "./repository";
import {
@ -34,8 +34,8 @@ test.beforeEach(() => {
});
const uploadToUploadsDomainFlags = createFeatureFlags([
"database_uploads_enabled",
"uploads_domain_enabled",
FeatureFlag.DatabaseUploadsEnabled,
FeatureFlag.UploadsDomainEnabled,
]);
const testRepoName: RepositoryNwo = { owner: "github", repo: "example" };
@ -70,7 +70,7 @@ async function mockHttpRequests(
const requestSpy = sinon.stub(client, "request");
const url = (await featureFlags.getUploadsDomainEnabled())
const url = (await featureFlags.getValue(FeatureFlag.UploadsDomainEnabled))
? "POST https://uploads.github.com/repos/:owner/:repo/code-scanning/codeql/databases/:language?name=:name"
: "PUT /repos/:owner/:repo/code-scanning/codeql/databases/:language";
const databaseUploadSpy = requestSpy.withArgs(url);
@ -218,7 +218,7 @@ test("Abort database upload if feature flag is disabled", async (t) => {
await uploadDatabases(
testRepoName,
getTestConfig(tmpDir),
createFeatureFlags(["uploads_domain_enabled"]),
createFeatureFlags([FeatureFlag.UploadsDomainEnabled]),
testApiDetails,
getRecordingLogger(loggedMessages)
);
@ -242,7 +242,9 @@ test("Don't crash if uploading a database fails", async (t) => {
.returns("true");
sinon.stub(actionsUtil, "isAnalyzingDefaultBranch").resolves(true);
const featureFlags = createFeatureFlags(["database_uploads_enabled"]);
const featureFlags = createFeatureFlags([
FeatureFlag.DatabaseUploadsEnabled,
]);
await mockHttpRequests(featureFlags, 500);
@ -261,8 +263,6 @@ test("Don't crash if uploading a database fails", async (t) => {
getRecordingLogger(loggedMessages)
);
console.log(loggedMessages);
t.assert(
loggedMessages.find(
(v) =>

View file

@ -4,7 +4,7 @@ import * as actionsUtil from "./actions-util";
import { getApiClient, GitHubApiDetails } from "./api-client";
import { getCodeQL } from "./codeql";
import { Config } from "./config-utils";
import { FeatureFlags } from "./feature-flags";
import { FeatureFlag, FeatureFlags } from "./feature-flags";
import { Logger } from "./logging";
import { RepositoryNwo } from "./repository";
import * as util from "./util";
@ -34,7 +34,7 @@ export async function uploadDatabases(
return;
}
if (!(await featureFlags.getDatabaseUploadsEnabled())) {
if (!(await featureFlags.getValue(FeatureFlag.DatabaseUploadsEnabled))) {
logger.debug(
"Repository is not opted in to database uploads. Skipping upload."
);
@ -43,7 +43,9 @@ export async function uploadDatabases(
const client = getApiClient(apiDetails);
const codeql = await getCodeQL(config.codeQLCmd);
const useUploadDomain = await featureFlags.getUploadsDomainEnabled();
const useUploadDomain = await featureFlags.getValue(
FeatureFlag.UploadsDomainEnabled
);
for (const language of config.languages) {
// Upload the database bundle.

View file

@ -4,7 +4,7 @@ import * as sinon from "sinon";
import * as apiClient from "./api-client";
import { GitHubApiDetails } from "./api-client";
import { GitHubFeatureFlags } from "./feature-flags";
import { FeatureFlag, GitHubFeatureFlags } from "./feature-flags";
import { getRunnerLogger } from "./logging";
import { parseRepositoryNwo } from "./repository";
import {
@ -85,9 +85,9 @@ for (const variant of ALL_FEATURE_FLAGS_DISABLED_VARIANTS) {
getRecordingLogger(loggedMessages)
);
t.assert((await featureFlags.getDatabaseUploadsEnabled()) === false);
t.assert((await featureFlags.getMlPoweredQueriesEnabled()) === false);
t.assert((await featureFlags.getUploadsDomainEnabled()) === false);
for (const flag of Object.values(FeatureFlag)) {
t.assert((await featureFlags.getValue(flag)) === false);
}
t.assert(
loggedMessages.find(
@ -115,9 +115,9 @@ test("Feature flags are disabled if they're not returned in API response", async
mockHttpRequests(200, {});
t.assert((await featureFlags.getDatabaseUploadsEnabled()) === false);
t.assert((await featureFlags.getMlPoweredQueriesEnabled()) === false);
t.assert((await featureFlags.getUploadsDomainEnabled()) === false);
for (const flag of Object.values(FeatureFlag)) {
t.assert((await featureFlags.getValue(flag)) === false);
}
for (const featureFlag of [
"database_uploads_enabled",
@ -182,11 +182,15 @@ for (const featureFlag of FEATURE_FLAGS) {
mockHttpRequests(200, expectedFeatureFlags);
const actualFeatureFlags = {
database_uploads_enabled:
await featureFlags.getDatabaseUploadsEnabled(),
ml_powered_queries_enabled:
await featureFlags.getMlPoweredQueriesEnabled(),
uploads_domain_enabled: await featureFlags.getUploadsDomainEnabled(),
database_uploads_enabled: await featureFlags.getValue(
FeatureFlag.DatabaseUploadsEnabled
),
ml_powered_queries_enabled: await featureFlags.getValue(
FeatureFlag.MlPoweredQueriesEnabled
),
uploads_domain_enabled: await featureFlags.getValue(
FeatureFlag.UploadsDomainEnabled
),
};
t.deepEqual(actualFeatureFlags, expectedFeatureFlags);

View file

@ -4,9 +4,13 @@ import { RepositoryNwo } from "./repository";
import * as util from "./util";
export interface FeatureFlags {
getDatabaseUploadsEnabled(): Promise<boolean>;
getMlPoweredQueriesEnabled(): Promise<boolean>;
getUploadsDomainEnabled(): Promise<boolean>;
getValue(flag: FeatureFlag): Promise<boolean>;
}
export enum FeatureFlag {
DatabaseUploadsEnabled = "database_uploads_enabled",
MlPoweredQueriesEnabled = "ml_powered_queries_enabled",
UploadsDomainEnabled = "uploads_domain_enabled",
}
/**
@ -15,7 +19,7 @@ export interface FeatureFlags {
*
* It maps feature flags to whether they are enabled or not.
*/
type FeatureFlagsApiResponse = { [flagName: string]: boolean };
type FeatureFlagsApiResponse = Partial<Record<FeatureFlag, boolean>>;
export class GitHubFeatureFlags implements FeatureFlags {
private cachedApiResponse: FeatureFlagsApiResponse | undefined;
@ -27,32 +31,21 @@ export class GitHubFeatureFlags implements FeatureFlags {
private logger: Logger
) {}
getDatabaseUploadsEnabled(): Promise<boolean> {
return this.getFeatureFlag("database_uploads_enabled");
}
getMlPoweredQueriesEnabled(): Promise<boolean> {
return this.getFeatureFlag("ml_powered_queries_enabled");
}
getUploadsDomainEnabled(): Promise<boolean> {
return this.getFeatureFlag("uploads_domain_enabled");
async getValue(flag: FeatureFlag): Promise<boolean> {
const response = (await this.getApiResponse())[flag];
if (response === undefined) {
this.logger.debug(
`Feature flag '${flag}' undefined in API response, considering it disabled.`
);
return false;
}
return response;
}
async preloadFeatureFlags(): Promise<void> {
await this.getApiResponse();
}
private async getFeatureFlag(name: string): Promise<boolean> {
const response = (await this.getApiResponse())[name];
if (response === undefined) {
this.logger.debug(
`Feature flag '${name}' undefined in API response, considering it disabled.`
);
}
return response || false;
}
private async getApiResponse(): Promise<FeatureFlagsApiResponse> {
const loadApiResponse = async () => {
// Do nothing when not running against github.com
@ -89,28 +82,15 @@ export class GitHubFeatureFlags implements FeatureFlags {
}
}
type FeatureFlagName =
| "database_uploads_enabled"
| "ml_powered_queries_enabled"
| "uploads_domain_enabled";
/**
* Create a feature flags instance with the specified set of enabled flags.
*
* This should be only used within tests.
*/
export function createFeatureFlags(
enabledFlags: FeatureFlagName[]
): FeatureFlags {
export function createFeatureFlags(enabledFlags: FeatureFlag[]): FeatureFlags {
return {
getDatabaseUploadsEnabled: async () => {
return enabledFlags.includes("database_uploads_enabled");
},
getMlPoweredQueriesEnabled: async () => {
return enabledFlags.includes("ml_powered_queries_enabled");
},
getUploadsDomainEnabled: async () => {
return enabledFlags.includes("uploads_domain_enabled");
getValue: async (flag) => {
return enabledFlags.includes(flag);
},
};
}