From a510ab7a872f6ffdd6b8ff3b210cdd4f20db5ecb Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:30:09 +0400 Subject: [PATCH 01/10] Use another ts proto API for griffin UI & tracker --- src/core/serializers.ts | 18 +++---- src/core/study_processor.ts | 47 +++++++++---------- src/core/summary.ts | 6 +-- src/finch_tracker/main.ts | 6 +-- src/finch_tracker/tracker_lib.test.ts | 15 +++--- src/finch_tracker/tracker_lib.ts | 7 +-- src/scripts/generate_proto_ts.ts | 34 ++------------ src/seed_tools/utils/seed_validation.test.ts | 10 ++-- src/seed_tools/utils/study_json_utils.test.ts | 6 +-- src/seed_tools/utils/study_validation.test.ts | 18 +++---- src/web/app/app.tsx | 10 ++-- src/web/app/experiment_model.ts | 8 ++-- src/web/app/seed_loader.ts | 4 +- src/web/app/study_model.test.ts | 23 +++++---- src/web/app/study_model.ts | 10 ++-- 15 files changed, 102 insertions(+), 120 deletions(-) diff --git a/src/core/serializers.ts b/src/core/serializers.ts index 034c5db1..01cdd095 100644 --- a/src/core/serializers.ts +++ b/src/core/serializers.ts @@ -2,7 +2,7 @@ // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -import { variations as proto } from '../proto/generated/proto_bundle'; +import {Study, Study_Channel, Study_Platform} from '../proto/generated/study'; export function getPlatformNameFromString(protoPlatfrom: string): string { const PREFIX = 'PLATFORM_'; @@ -11,8 +11,8 @@ export function getPlatformNameFromString(protoPlatfrom: string): string { return protoPlatfrom; } -export function getPlatfromName(protoPlatfrom: proto.Study.Platform): string { - return getPlatformNameFromString(proto.Study.Platform[protoPlatfrom]); +export function getPlatfromName(protoPlatfrom: Study_Platform): string { + return getPlatformNameFromString(Study_Platform[protoPlatfrom]); } export function getChannelNameFromString( @@ -27,11 +27,11 @@ export function getChannelNameFromString( } export function getChannelName( - protoChannel: proto.Study.Channel, + protoChannel: Study_Channel, isBraveSpecific: boolean, ): string { return getChannelNameFromString( - proto.Study.Channel[protoChannel], + Study_Channel[protoChannel], isBraveSpecific, ); } @@ -52,9 +52,11 @@ export function serializeChannels(channels?: string[]): string | undefined { // Converts a study to JSON that is ready to be serialized. Some field are // removed, some are converted to a human readable format. -export function studyToJSON(study: proto.IStudy): Record { - const msg = proto.Study.fromObject(study); - const json = msg.toJSON(); +export function studyToJSON(study: Study): Record { + const json = Study.toJson(study) as Record | null; + if (json === null) { + throw new Error('Failed to convert study to JSON'); + } const filter = json.filter; delete json.consistency; delete json.activation_type; diff --git a/src/core/study_processor.ts b/src/core/study_processor.ts index 77d2039b..36732fed 100644 --- a/src/core/study_processor.ts +++ b/src/core/study_processor.ts @@ -2,20 +2,20 @@ // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -import { variations as proto } from '../proto/generated/proto_bundle'; +import { Study, Study_Channel, Study_Experiment, Study_Filter, Study_Platform} from '../proto/generated/study'; import { type ProcessingOptions } from './base_types'; import { isFeatureBlocklisted, isStudyNameBlocklisted } from './blocklists'; import { matchesMaxVersion, parseVersionPattern } from './version'; -const UPSTREAM_SUPPORTED_PLATFORMS: readonly proto.Study.Platform[] = [ - proto.Study.Platform.PLATFORM_ANDROID, - proto.Study.Platform.PLATFORM_LINUX, - proto.Study.Platform.PLATFORM_MAC, - proto.Study.Platform.PLATFORM_WINDOWS, +const UPSTREAM_SUPPORTED_PLATFORMS: readonly Study_Platform[] = [ + Study_Platform.ANDROID, + Study_Platform.LINUX, + Study_Platform.MAC, + Study_Platform.WINDOWS, ]; -const BRAVE_SUPPORTED_PLATFORMS: readonly proto.Study.Platform[] = - UPSTREAM_SUPPORTED_PLATFORMS.concat([proto.Study.Platform.PLATFORM_IOS]); +const BRAVE_SUPPORTED_PLATFORMS: readonly Study_Platform[] = + UPSTREAM_SUPPORTED_PLATFORMS.concat([Study_Platform.IOS]); export enum StudyChannelTarget { // filter.channel includes DEV or CANNERY, doesn't include STABLE or BETA. @@ -86,14 +86,14 @@ export class StudyFilter { } } -// A wrapper over a raw proto.Study that processes it and collects some extra +// A wrapper over a raw Study that processes it and collects some extra // data. export class ProcessedStudy { - study: proto.IStudy; + study: Study; studyDetails: StudyDetails; affectedFeatures: Set; - constructor(study: proto.IStudy, options: ProcessingOptions) { + constructor(study: Study, options: ProcessingOptions) { this.study = study; this.studyDetails = new StudyDetails(study, options); this.affectedFeatures = getAffectedFeatures(study); @@ -106,7 +106,7 @@ export class ProcessedStudy { stripEmptyFilterGroups(): void { this.study.experiment = this.study.experiment?.filter( - (e) => e.probability_weight > 0, + (e) => (e.probability_weight ?? 0) > 0, ); } @@ -172,7 +172,7 @@ export class StudyDetails { maxNonDefaultIndex = -1; channelTarget = StudyChannelTarget.DEV_OR_CANARY; - constructor(study: proto.IStudy, options: ProcessingOptions) { + constructor(study: Study, options: ProcessingOptions) { const filter = study.filter; const experiment = study.experiment; const maxVersion = filter?.max_version; @@ -202,7 +202,7 @@ export class StudyDetails { endDateSeconds = filter.end_date; } else { // Long - endDateSeconds = filter.end_date.toNumber(); + endDateSeconds = Number(filter.end_date); } if ( @@ -227,7 +227,7 @@ export class StudyDetails { this.isBlocklisted ||= disabledFeatures?.some((n) => isFeatureBlocklisted(n)) ?? false; - this.isKillSwitch ||= e.probability_weight > 0 && isKillSwitch(e.name); + this.isKillSwitch ||= (e.probability_weight ?? 0) > 0 && isKillSwitch(e.name); this.onlyDisabledFeatures &&= e.probability_weight === 0 || @@ -242,7 +242,7 @@ export class StudyDetails { let index = 0; for (const e of experiment) { - const weight = e.probability_weight; + const weight = e.probability_weight ?? 0; this.totalWeight += weight; if ( !e.name.startsWith('Default') && @@ -259,9 +259,9 @@ export class StudyDetails { } const channel = study.filter?.channel; - if (channel?.includes(proto.Study.Channel.BETA)) + if (channel?.includes(Study_Channel.BETA)) this.channelTarget = StudyChannelTarget.BETA; - if (channel?.includes(proto.Study.Channel.STABLE)) + if (channel?.includes(Study_Channel.STABLE)) this.channelTarget = StudyChannelTarget.STABLE; } @@ -293,7 +293,7 @@ export class StudyDetails { } } -function getAffectedFeatures(study: proto.IStudy): Set { +function getAffectedFeatures(study: Study): Set { const features = new Set(); const experiment = study.experiment; if (experiment == null) { @@ -306,7 +306,7 @@ function getAffectedFeatures(study: proto.IStudy): Set { return features; } -function areFeaturesInDefaultStates(e: proto.Study.IExperiment): boolean { +function areFeaturesInDefaultStates(e: Study_Experiment): boolean { const enableFeature = e.feature_association?.enable_feature; const disableFeature = e.feature_association?.disable_feature; if (enableFeature != null && enableFeature.length > 0) return false; @@ -315,11 +315,10 @@ function areFeaturesInDefaultStates(e: proto.Study.IExperiment): boolean { } function filterPlatforms( - f: proto.Study.IFilter | undefined | null, + f: Study_Filter, isBraveSeed: boolean, -): proto.Study.Platform[] | undefined { +): Study_Platform[] { const platform = f?.platform; - if (platform == null) return undefined; const supportedPlatforms = isBraveSeed ? BRAVE_SUPPORTED_PLATFORMS : UPSTREAM_SUPPORTED_PLATFORMS; @@ -328,7 +327,7 @@ function filterPlatforms( // Processes a list of studies and groups it according to study.name. export function processStudyList( - list: proto.IStudy[], + list: Study[], minPriority: StudyPriority, options: ProcessingOptions, ): Map { diff --git a/src/core/summary.ts b/src/core/summary.ts index e403c260..117e2491 100644 --- a/src/core/summary.ts +++ b/src/core/summary.ts @@ -4,7 +4,6 @@ // You can obtain one at https://mozilla.org/MPL/2.0/. import { createHash } from 'node:crypto'; -import { type variations as proto } from '../proto/generated/proto_bundle'; import { SeedType, type ProcessingOptions } from './base_types'; import { StudyPriority, @@ -15,6 +14,7 @@ import { import * as config from '../config'; import * as url_utils from './url_utils'; +import { VariationsSeed } from '../proto/generated/variations_seed'; export enum ItemAction { New, @@ -115,8 +115,8 @@ function getOverallAudience( } export function makeSummary( - oldSeed: proto.VariationsSeed, - newSeed: proto.VariationsSeed, + oldSeed: VariationsSeed, + newSeed: VariationsSeed, options: ProcessingOptions, minPriority: StudyPriority, ): Map { diff --git a/src/finch_tracker/main.ts b/src/finch_tracker/main.ts index 9eba43d2..4c360881 100644 --- a/src/finch_tracker/main.ts +++ b/src/finch_tracker/main.ts @@ -8,13 +8,13 @@ import { type ProcessingOptions } from '../core/base_types'; import { StudyPriority } from '../core/study_processor'; import { makeSummary, summaryToJson } from '../core/summary'; import * as url_utils from '../core/url_utils'; -import { variations as proto } from '../proto/generated/proto_bundle'; import { downloadUrl, getSeedPath } from './node_utils'; import { commitAllChanges, fetchChromeSeedData, storeDataToDirectory, } from './tracker_lib'; +import { VariationsSeed } from '../proto/generated/variations_seed'; async function main(): Promise { const program = new Command() @@ -86,7 +86,7 @@ async function main(): Promise { seedFile !== undefined ? fs.readFileSync(seedFile) : await fetchChromeSeedData(); - const seed = proto.VariationsSeed.decode(seedData); + const seed = VariationsSeed.fromBinary(seedData); let previousSeedData: Buffer | undefined; let newGitSha1: string | undefined; @@ -105,7 +105,7 @@ async function main(): Promise { } if (createSummary && previousSeedData !== undefined) { - const previousSeed = proto.VariationsSeed.decode(previousSeedData); + const previousSeed = VariationsSeed.fromBinary(previousSeedData); const summary = makeSummary( previousSeed, seed, diff --git a/src/finch_tracker/tracker_lib.test.ts b/src/finch_tracker/tracker_lib.test.ts index 9a9d6a95..4e7c8278 100644 --- a/src/finch_tracker/tracker_lib.test.ts +++ b/src/finch_tracker/tracker_lib.test.ts @@ -8,7 +8,8 @@ import * as fs from 'fs'; import { describe, expect, test } from '@jest/globals'; import { StudyPriority } from '../core/study_processor'; import { ItemAction, makeSummary, summaryToJson } from '../core/summary'; -import { variations as proto } from '../proto/generated/proto_bundle'; +import { Study, Study_Channel, Study_Platform} from '../proto/generated/study'; +import { VariationsSeed } from '../proto/generated/variations_seed'; import { serializeStudies } from './tracker_lib'; function serialize(json: Record) { @@ -46,11 +47,11 @@ describe('summary', () => { const common = { name: 'TestStudy', filter: { - channel: [proto.Study.Channel.STABLE], - platform: [proto.Study.Platform.PLATFORM_WINDOWS], + channel: [Study_Channel.STABLE], + platform: [Study_Platform.WINDOWS], }, }; - const oldStudy = new proto.Study({ + const oldStudy = Study.create({ ...common, experiment: [ { @@ -65,7 +66,7 @@ describe('summary', () => { ], }); - const newStudy = new proto.Study({ + const newStudy = Study.create({ ...common, experiment: [ { @@ -84,8 +85,8 @@ describe('summary', () => { }, ], }); - const oldSeed = new proto.VariationsSeed({ study: [oldStudy] }); - const newSeed = new proto.VariationsSeed({ study: [newStudy] }); + const oldSeed = VariationsSeed.create({ study: [oldStudy] }); + const newSeed = VariationsSeed.create({ study: [newStudy] }); const summary = makeSummary( oldSeed, diff --git a/src/finch_tracker/tracker_lib.ts b/src/finch_tracker/tracker_lib.ts index f8af09be..7c1149a6 100644 --- a/src/finch_tracker/tracker_lib.ts +++ b/src/finch_tracker/tracker_lib.ts @@ -14,7 +14,8 @@ import { StudyPriority, priorityToText, } from '../core/study_processor'; -import { variations as proto } from '../proto/generated/proto_bundle'; +import { Study } from '../proto/generated/study'; +import { VariationsSeed } from '../proto/generated/variations_seed'; import { downloadUrl, getSeedPath, getStudyPath } from './node_utils'; export async function fetchChromeSeedData(): Promise { @@ -29,8 +30,8 @@ export function serializeStudies( options: ProcessingOptions, ): Record { const map: Record = {}; - const seed = proto.VariationsSeed.decode(seedData); - const addStudy = (path: string, study: proto.IStudy) => { + const seed = VariationsSeed.fromBinary(seedData); + const addStudy = (path: string, study: Study) => { const json = studyToJSON(study); const list = map[path]; if (list !== undefined) list.push(json); diff --git a/src/scripts/generate_proto_ts.ts b/src/scripts/generate_proto_ts.ts index a1c541c5..9d8408ad 100644 --- a/src/scripts/generate_proto_ts.ts +++ b/src/scripts/generate_proto_ts.ts @@ -36,7 +36,6 @@ function main(options: Options) { } removeGeneratedFiles(); - generateProtobufJsWithTypeInfo(); generateProtobufTs(); } @@ -49,33 +48,6 @@ function removeGeneratedFiles() { }); } -function generateProtobufJsWithTypeInfo() { - execSync( - [ - 'npx', - '--', - 'pbjs', - '--t', - 'static-module', - '--keep-case', - ...protoFiles, - '-o', - `${protoGeneratedDir}/proto_bundle.js`, - ].join(' '), - ); - - execSync( - [ - 'npx', - '--', - 'pbts', - '-o', - `${protoGeneratedDir}/proto_bundle.d.ts`, - `${protoGeneratedDir}/proto_bundle.js`, - ].join(' '), - ); -} - function generateProtobufTs() { try { // Apply study.proto patch to make protobuf-ts serialize probability_weight @@ -102,7 +74,7 @@ function generateProtobufTs() { function generateStudyProtoPatch() { fs.writeFileSync( - `${protoDir}/study.proto.protobuf-ts.patch`, + `${protoDir}/study.protobuf-ts.patch`, execSync(`git diff ${protoDir}/study.proto`, { encoding: 'buffer', }), @@ -110,14 +82,14 @@ function generateStudyProtoPatch() { } function gitApplyStudyProtoPatch() { - execSync(`git apply ${protoDir}/study.proto.protobuf-ts.patch`, { + execSync(`git apply ${protoDir}/study.protobuf-ts.patch`, { cwd: protoDir, stdio: 'inherit', }); } function gitRevertStudyProtoPatch() { - execSync(`git apply -R ${protoDir}/study.proto.protobuf-ts.patch`, { + execSync(`git apply -R ${protoDir}/study.protobuf-ts.patch`, { cwd: protoDir, stdio: 'inherit', }); diff --git a/src/seed_tools/utils/seed_validation.test.ts b/src/seed_tools/utils/seed_validation.test.ts index 3a4ef2fb..ff215233 100644 --- a/src/seed_tools/utils/seed_validation.test.ts +++ b/src/seed_tools/utils/seed_validation.test.ts @@ -31,7 +31,7 @@ describe('getSeedErrors', () => { } else { study.filter = {}; } - study.filter.platform = study.filter.platform ?? ['PLATFORM_LINUX']; + study.filter.platform = study.filter.platform ?? ['LINUX']; study.filter.channel = study.filter.channel ?? ['BETA']; return Study.fromJson(study, { ignoreUnknownFields: false, @@ -184,19 +184,19 @@ describe('getSeedErrors', () => { // platform tests { filter1: { - platform: ['PLATFORM_WINDOWS', 'PLATFORM_MAC'], + platform: ['WINDOWS', 'MAC'], }, filter2: { - platform: ['PLATFORM_MAC'], + platform: ['MAC'], }, expectedOverlapped: true, }, { filter1: { - platform: ['PLATFORM_WINDOWS'], + platform: ['WINDOWS'], }, filter2: { - platform: ['PLATFORM_MAC'], + platform: ['MAC'], }, expectedOverlapped: false, }, diff --git a/src/seed_tools/utils/study_json_utils.test.ts b/src/seed_tools/utils/study_json_utils.test.ts index 9c29d313..b645bce1 100644 --- a/src/seed_tools/utils/study_json_utils.test.ts +++ b/src/seed_tools/utils/study_json_utils.test.ts @@ -44,7 +44,7 @@ describe('stringifyStudies', () => { name: 'study', filter: { channel: ['CANARY', 'BETA', 'STABLE'], - platform: ['PLATFORM_LINUX', 'PLATFORM_MAC'], + platform: ['LINUX', 'MAC'], }, }, { ignoreUnknownFields: false }, @@ -112,7 +112,7 @@ describe('stringifyStudies', () => { filter: { start_date: Math.floor(startDate.getTime() / 1000), channel: ['CANARY', 'BETA', 'STABLE'], - platform: ['PLATFORM_LINUX', 'PLATFORM_MAC'], + platform: ['LINUX', 'MAC'], }, }, { ignoreUnknownFields: false }, @@ -127,7 +127,7 @@ describe('stringifyStudies', () => { filter: { start_date: startDate.toISOString(), channel: ['CANARY', 'BETA', 'STABLE'], - platform: ['PLATFORM_LINUX', 'PLATFORM_MAC'], + platform: ['LINUX', 'MAC'], }, }, ]); diff --git a/src/seed_tools/utils/study_validation.test.ts b/src/seed_tools/utils/study_validation.test.ts index 4805f295..842154dd 100644 --- a/src/seed_tools/utils/study_validation.test.ts +++ b/src/seed_tools/utils/study_validation.test.ts @@ -30,7 +30,7 @@ describe('getStudyErrors', () => { filter: { locale: ['en'], channel: ['CANARY'], - platform: ['PLATFORM_WINDOWS'], + platform: ['WINDOWS'], start_date: Math.floor(new Date('2022-01-01').getTime() / 1000), end_date: Math.floor(new Date('2022-02-01').getTime() / 1000), min_version: '1.0', @@ -141,7 +141,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['PLATFORM_LINUX'], + platform: ['LINUX'], channel: ['BETA'], }, }); @@ -263,7 +263,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['PLATFORM_LINUX'], + platform: ['LINUX'], channel: ['BETA'], }, }); @@ -286,7 +286,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['PLATFORM_LINUX'], + platform: ['LINUX'], channel: ['BETA'], }, }); @@ -362,7 +362,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['PLATFORM_LINUX'], + platform: ['LINUX'], channel: ['BETA'], }, }; @@ -543,7 +543,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['PLATFORM_LINUX'], + platform: ['LINUX'], channel: ['BETA'], locale: [], exclude_locale: ['en'], @@ -634,7 +634,7 @@ describe('getStudyErrors', () => { probability_weight: 100, }, ], - filter: { ...filter, platform: ['PLATFORM_LINUX'], channel: ['BETA'] }, + filter: { ...filter, platform: ['LINUX'], channel: ['BETA'] }, }); expect(study_validation.getStudyErrors(study, studyFileBaseName)).toEqual( @@ -693,7 +693,7 @@ describe('getStudyErrors', () => { probability_weight: 100, }, ], - filter: { ...filter, platform: ['PLATFORM_LINUX'] }, + filter: { ...filter, platform: ['LINUX'] }, }); expect( @@ -705,7 +705,7 @@ describe('getStudyErrors', () => { test.each([ {}, { platform: [] }, - { platform: ['PLATFORM_LINUX', 'PLATFORM_LINUX'] }, + { platform: ['LINUX', 'LINUX'] }, ])('should error if platform is invalid', (filter: any) => { const study = Study.fromJson({ name: 'study', diff --git a/src/web/app/app.tsx b/src/web/app/app.tsx index b310b520..85fb84e1 100644 --- a/src/web/app/app.tsx +++ b/src/web/app/app.tsx @@ -12,7 +12,7 @@ import { priorityToText, type StudyFilter, } from '../../core/study_processor'; -import { variations as proto } from '../../proto/generated/proto_bundle'; +import { Study_CpuArchitecture, Study_FormFactor } from '../../proto/generated/study'; import { type ExperimentModel, type FeatureModel } from './experiment_model'; import { SearchParamManager } from './search_param_manager'; import { loadSeedDataAsync } from './seed_loader'; @@ -216,18 +216,18 @@ export function StudyItem(props: { study: StudyModel; filter: StudyFilter }) { /> proto.Study.FormFactor[e])} + include={filter?.form_factor?.map((e) => Study_FormFactor[e])} exclude={filter?.exclude_form_factor?.map( - (e) => proto.Study.FormFactor[e], + (e) => Study_FormFactor[e], )} /> proto.Study.CpuArchitecture[e], + (e) => Study_CpuArchitecture[e], )} exclude={filter?.exclude_cpu_architecture?.map( - (e) => proto.Study.CpuArchitecture[e], + (e) => Study_CpuArchitecture[e], )} /> {filter != null && diff --git a/src/web/app/experiment_model.ts b/src/web/app/experiment_model.ts index 1efacaef..882f8f6b 100644 --- a/src/web/app/experiment_model.ts +++ b/src/web/app/experiment_model.ts @@ -4,7 +4,7 @@ // You can obtain one at https://mozilla.org/MPL/2.0/. import * as url_utils from '../../core/url_utils'; -import { type variations as proto } from '../../proto/generated/proto_bundle'; +import { Study_Experiment } from '../../proto/generated/study'; import { type StudyModel } from './study_model'; export class FeatureModel { @@ -13,10 +13,10 @@ export class FeatureModel { } export class ExperimentModel { - private readonly experiment: proto.Study.IExperiment; + private readonly experiment: Study_Experiment; private readonly studyModel: StudyModel; - constructor(experiment: proto.Study.IExperiment, studyModel: StudyModel) { + constructor(experiment: Study_Experiment, studyModel: StudyModel) { this.experiment = experiment; this.studyModel = studyModel; } @@ -58,7 +58,7 @@ export class ExperimentModel { weight(): number { const totalWeight = this.studyModel.processedStudy.studyDetails.totalWeight; if (totalWeight === 0) return 0; - return (this.experiment.probability_weight / totalWeight) * 100; + return ((this.experiment.probability_weight ?? 0) / totalWeight) * 100; } isMajorGroup(): boolean { diff --git a/src/web/app/seed_loader.ts b/src/web/app/seed_loader.ts index e79deb1c..fb04f2c1 100644 --- a/src/web/app/seed_loader.ts +++ b/src/web/app/seed_loader.ts @@ -5,8 +5,8 @@ import { SeedType, type ProcessingOptions } from '../../core/base_types'; import { ProcessedStudy } from '../../core/study_processor'; -import { variations as proto } from '../../proto/generated/proto_bundle'; import { StudyListModel, StudyModel } from './study_model'; +import { VariationsSeed } from '../../proto/generated/variations_seed'; import * as url_utils from '../../core/url_utils'; @@ -47,7 +47,7 @@ async function loadFile( async function loadSeedFromUrl(url: string, type: SeedType) { const data = await loadFile(url, 'arraybuffer'); const seedBytes = new Uint8Array(data); - const seed = proto.VariationsSeed.decode(seedBytes); + const seed = VariationsSeed.fromBinary(seedBytes); const isBraveSeed = type !== SeedType.UPSTREAM; // Desktop/Android could use a different major chrome version. diff --git a/src/web/app/study_model.test.ts b/src/web/app/study_model.test.ts index 9431faa6..22553ef8 100644 --- a/src/web/app/study_model.test.ts +++ b/src/web/app/study_model.test.ts @@ -10,11 +10,12 @@ import { StudyFilter, StudyPriority, } from '../../core/study_processor'; -import { variations as proto } from '../../proto/generated/proto_bundle'; import { StudyListModel, StudyModel } from './study_model'; +import { Study, Study_Channel, Study_Platform } from '../../proto/generated/study'; +import { PartialMessage } from '@protobuf-ts/runtime'; -function makeStudyModel(properties: proto.IStudy) { - const study = new proto.Study(properties); +function makeStudyModel(properties: PartialMessage) { + const study = Study.create(properties); const processed = new ProcessedStudy(study, { minMajorVersion: 116, isBraveSeed: true, @@ -46,10 +47,10 @@ describe('models', () => { }, ], filter: { - channel: [proto.Study.Channel.STABLE, proto.Study.Channel.CANARY], + channel: [Study_Channel.STABLE, Study_Channel.CANARY], platform: [ - proto.Study.Platform.PLATFORM_WINDOWS, - proto.Study.Platform.PLATFORM_IOS, + Study_Platform.WINDOWS, + Study_Platform.IOS, ], }, }); @@ -60,11 +61,13 @@ describe('models', () => { { name: 'Some', probability_weight: 100, + param: [], + override_ui_string: [] }, ], filter: { - channel: [proto.Study.Channel.STABLE, proto.Study.Channel.CANARY], - platform: [proto.Study.Platform.PLATFORM_WINDOWS], + channel: [Study_Channel.STABLE, Study_Channel.CANARY], + platform: [Study_Platform.WINDOWS], max_version: '110.0.0.0', }, }); @@ -78,8 +81,8 @@ describe('models', () => { }, ], filter: { - channel: [proto.Study.Channel.BETA], - platform: [proto.Study.Platform.PLATFORM_WINDOWS], + channel: [Study_Channel.BETA], + platform: [Study_Platform.WINDOWS], }, }); diff --git a/src/web/app/study_model.ts b/src/web/app/study_model.ts index 1300d299..46b016e6 100644 --- a/src/web/app/study_model.ts +++ b/src/web/app/study_model.ts @@ -11,7 +11,7 @@ import { type StudyPriority, } from '../../core/study_processor'; import * as url_utils from '../../core/url_utils'; -import { type variations as proto } from '../../proto/generated/proto_bundle'; +import { Study_Filter } from '../../proto/generated/study'; import { ExperimentModel } from './experiment_model'; export class StudyModel { @@ -25,7 +25,7 @@ export class StudyModel { this.id = id; } - filter(): proto.Study.IFilter | undefined { + filter(): Study_Filter | undefined { return this.processedStudy.study.filter ?? undefined; } @@ -42,7 +42,11 @@ export class StudyModel { if (study.experiment == null) return []; const models: ExperimentModel[] = []; for (const e of study.experiment) { - if (e.probability_weight > 0 || f === undefined || f.showEmptyGroups) { + if ( + (e.probability_weight ?? 0) > 0 || + f === undefined || + f.showEmptyGroups + ) { const model = new ExperimentModel(e, this); models.push(model); } From ce1cb041aa3cdb63a8d9118a1faa8cfc280af282 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:32:08 +0400 Subject: [PATCH 02/10] Fix lint --- src/core/serializers.ts | 7 +--- src/core/study_processor.ts | 11 +++++- src/core/summary.ts | 2 +- src/finch_tracker/main.ts | 2 +- src/finch_tracker/tracker_lib.test.ts | 2 +- src/seed_tools/utils/study_validation.test.ts | 37 +++++++++---------- src/web/app/app.tsx | 9 +++-- src/web/app/seed_loader.ts | 2 +- src/web/app/study_model.test.ts | 15 ++++---- 9 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/core/serializers.ts b/src/core/serializers.ts index 01cdd095..05310c19 100644 --- a/src/core/serializers.ts +++ b/src/core/serializers.ts @@ -2,7 +2,7 @@ // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -import {Study, Study_Channel, Study_Platform} from '../proto/generated/study'; +import { Study, Study_Channel, Study_Platform } from '../proto/generated/study'; export function getPlatformNameFromString(protoPlatfrom: string): string { const PREFIX = 'PLATFORM_'; @@ -30,10 +30,7 @@ export function getChannelName( protoChannel: Study_Channel, isBraveSpecific: boolean, ): string { - return getChannelNameFromString( - Study_Channel[protoChannel], - isBraveSpecific, - ); + return getChannelNameFromString(Study_Channel[protoChannel], isBraveSpecific); } function unixSecondToUTCString(unixTimeSeconds: number): string { diff --git a/src/core/study_processor.ts b/src/core/study_processor.ts index 36732fed..31c5f851 100644 --- a/src/core/study_processor.ts +++ b/src/core/study_processor.ts @@ -2,7 +2,13 @@ // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -import { Study, Study_Channel, Study_Experiment, Study_Filter, Study_Platform} from '../proto/generated/study'; +import { + Study, + Study_Channel, + Study_Experiment, + Study_Filter, + Study_Platform, +} from '../proto/generated/study'; import { type ProcessingOptions } from './base_types'; import { isFeatureBlocklisted, isStudyNameBlocklisted } from './blocklists'; import { matchesMaxVersion, parseVersionPattern } from './version'; @@ -227,7 +233,8 @@ export class StudyDetails { this.isBlocklisted ||= disabledFeatures?.some((n) => isFeatureBlocklisted(n)) ?? false; - this.isKillSwitch ||= (e.probability_weight ?? 0) > 0 && isKillSwitch(e.name); + this.isKillSwitch ||= + (e.probability_weight ?? 0) > 0 && isKillSwitch(e.name); this.onlyDisabledFeatures &&= e.probability_weight === 0 || diff --git a/src/core/summary.ts b/src/core/summary.ts index 117e2491..a2ee2acc 100644 --- a/src/core/summary.ts +++ b/src/core/summary.ts @@ -13,8 +13,8 @@ import { } from './study_processor'; import * as config from '../config'; -import * as url_utils from './url_utils'; import { VariationsSeed } from '../proto/generated/variations_seed'; +import * as url_utils from './url_utils'; export enum ItemAction { New, diff --git a/src/finch_tracker/main.ts b/src/finch_tracker/main.ts index 4c360881..27bd543c 100644 --- a/src/finch_tracker/main.ts +++ b/src/finch_tracker/main.ts @@ -8,13 +8,13 @@ import { type ProcessingOptions } from '../core/base_types'; import { StudyPriority } from '../core/study_processor'; import { makeSummary, summaryToJson } from '../core/summary'; import * as url_utils from '../core/url_utils'; +import { VariationsSeed } from '../proto/generated/variations_seed'; import { downloadUrl, getSeedPath } from './node_utils'; import { commitAllChanges, fetchChromeSeedData, storeDataToDirectory, } from './tracker_lib'; -import { VariationsSeed } from '../proto/generated/variations_seed'; async function main(): Promise { const program = new Command() diff --git a/src/finch_tracker/tracker_lib.test.ts b/src/finch_tracker/tracker_lib.test.ts index 4e7c8278..8f57c577 100644 --- a/src/finch_tracker/tracker_lib.test.ts +++ b/src/finch_tracker/tracker_lib.test.ts @@ -8,7 +8,7 @@ import * as fs from 'fs'; import { describe, expect, test } from '@jest/globals'; import { StudyPriority } from '../core/study_processor'; import { ItemAction, makeSummary, summaryToJson } from '../core/summary'; -import { Study, Study_Channel, Study_Platform} from '../proto/generated/study'; +import { Study, Study_Channel, Study_Platform } from '../proto/generated/study'; import { VariationsSeed } from '../proto/generated/variations_seed'; import { serializeStudies } from './tracker_lib'; diff --git a/src/seed_tools/utils/study_validation.test.ts b/src/seed_tools/utils/study_validation.test.ts index 842154dd..c6f40099 100644 --- a/src/seed_tools/utils/study_validation.test.ts +++ b/src/seed_tools/utils/study_validation.test.ts @@ -702,24 +702,23 @@ describe('getStudyErrors', () => { }, ); - test.each([ - {}, - { platform: [] }, - { platform: ['LINUX', 'LINUX'] }, - ])('should error if platform is invalid', (filter: any) => { - const study = Study.fromJson({ - name: 'study', - experiment: [ - { - name: 'experiment1', - probability_weight: 100, - }, - ], - filter: { ...filter, channel: ['BETA'] }, - }); + test.each([{}, { platform: [] }, { platform: ['LINUX', 'LINUX'] }])( + 'should error if platform is invalid', + (filter: any) => { + const study = Study.fromJson({ + name: 'study', + experiment: [ + { + name: 'experiment1', + probability_weight: 100, + }, + ], + filter: { ...filter, channel: ['BETA'] }, + }); - expect( - study_validation.getStudyErrors(study, studyFileBaseName), - ).toContainEqual(expect.stringMatching(/(P|p)latform/)); - }); + expect( + study_validation.getStudyErrors(study, studyFileBaseName), + ).toContainEqual(expect.stringMatching(/(P|p)latform/)); + }, + ); }); diff --git a/src/web/app/app.tsx b/src/web/app/app.tsx index 85fb84e1..70af53ec 100644 --- a/src/web/app/app.tsx +++ b/src/web/app/app.tsx @@ -12,7 +12,10 @@ import { priorityToText, type StudyFilter, } from '../../core/study_processor'; -import { Study_CpuArchitecture, Study_FormFactor } from '../../proto/generated/study'; +import { + Study_CpuArchitecture, + Study_FormFactor, +} from '../../proto/generated/study'; import { type ExperimentModel, type FeatureModel } from './experiment_model'; import { SearchParamManager } from './search_param_manager'; import { loadSeedDataAsync } from './seed_loader'; @@ -217,9 +220,7 @@ export function StudyItem(props: { study: StudyModel; filter: StudyFilter }) { Study_FormFactor[e])} - exclude={filter?.exclude_form_factor?.map( - (e) => Study_FormFactor[e], - )} + exclude={filter?.exclude_form_factor?.map((e) => Study_FormFactor[e])} /> ) { const study = Study.create(properties); @@ -48,10 +52,7 @@ describe('models', () => { ], filter: { channel: [Study_Channel.STABLE, Study_Channel.CANARY], - platform: [ - Study_Platform.WINDOWS, - Study_Platform.IOS, - ], + platform: [Study_Platform.WINDOWS, Study_Platform.IOS], }, }); @@ -61,8 +62,6 @@ describe('models', () => { { name: 'Some', probability_weight: 100, - param: [], - override_ui_string: [] }, ], filter: { From b70d293d33c63080b3c93ad786c96c2730164f5e Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:36:06 +0400 Subject: [PATCH 03/10] Fix some tests --- src/core/serializers.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/serializers.ts b/src/core/serializers.ts index 05310c19..50f91001 100644 --- a/src/core/serializers.ts +++ b/src/core/serializers.ts @@ -50,7 +50,10 @@ export function serializeChannels(channels?: string[]): string | undefined { // Converts a study to JSON that is ready to be serialized. Some field are // removed, some are converted to a human readable format. export function studyToJSON(study: Study): Record { - const json = Study.toJson(study) as Record | null; + const json = Study.toJson(study, { useProtoFieldName: true }) as Record< + string, + any + > | null; if (json === null) { throw new Error('Failed to convert study to JSON'); } From 13ce725e9aecbe5a3a2400ad2d3c68ac0c92eb95 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:41:02 +0400 Subject: [PATCH 04/10] Revert seed_tools changes --- src/seed_tools/utils/seed_validation.test.ts | 10 ++-- src/seed_tools/utils/study_json_utils.test.ts | 6 +-- src/seed_tools/utils/study_validation.test.ts | 53 ++++++++++--------- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/seed_tools/utils/seed_validation.test.ts b/src/seed_tools/utils/seed_validation.test.ts index ff215233..3a4ef2fb 100644 --- a/src/seed_tools/utils/seed_validation.test.ts +++ b/src/seed_tools/utils/seed_validation.test.ts @@ -31,7 +31,7 @@ describe('getSeedErrors', () => { } else { study.filter = {}; } - study.filter.platform = study.filter.platform ?? ['LINUX']; + study.filter.platform = study.filter.platform ?? ['PLATFORM_LINUX']; study.filter.channel = study.filter.channel ?? ['BETA']; return Study.fromJson(study, { ignoreUnknownFields: false, @@ -184,19 +184,19 @@ describe('getSeedErrors', () => { // platform tests { filter1: { - platform: ['WINDOWS', 'MAC'], + platform: ['PLATFORM_WINDOWS', 'PLATFORM_MAC'], }, filter2: { - platform: ['MAC'], + platform: ['PLATFORM_MAC'], }, expectedOverlapped: true, }, { filter1: { - platform: ['WINDOWS'], + platform: ['PLATFORM_WINDOWS'], }, filter2: { - platform: ['MAC'], + platform: ['PLATFORM_MAC'], }, expectedOverlapped: false, }, diff --git a/src/seed_tools/utils/study_json_utils.test.ts b/src/seed_tools/utils/study_json_utils.test.ts index b645bce1..9c29d313 100644 --- a/src/seed_tools/utils/study_json_utils.test.ts +++ b/src/seed_tools/utils/study_json_utils.test.ts @@ -44,7 +44,7 @@ describe('stringifyStudies', () => { name: 'study', filter: { channel: ['CANARY', 'BETA', 'STABLE'], - platform: ['LINUX', 'MAC'], + platform: ['PLATFORM_LINUX', 'PLATFORM_MAC'], }, }, { ignoreUnknownFields: false }, @@ -112,7 +112,7 @@ describe('stringifyStudies', () => { filter: { start_date: Math.floor(startDate.getTime() / 1000), channel: ['CANARY', 'BETA', 'STABLE'], - platform: ['LINUX', 'MAC'], + platform: ['PLATFORM_LINUX', 'PLATFORM_MAC'], }, }, { ignoreUnknownFields: false }, @@ -127,7 +127,7 @@ describe('stringifyStudies', () => { filter: { start_date: startDate.toISOString(), channel: ['CANARY', 'BETA', 'STABLE'], - platform: ['LINUX', 'MAC'], + platform: ['PLATFORM_LINUX', 'PLATFORM_MAC'], }, }, ]); diff --git a/src/seed_tools/utils/study_validation.test.ts b/src/seed_tools/utils/study_validation.test.ts index c6f40099..4805f295 100644 --- a/src/seed_tools/utils/study_validation.test.ts +++ b/src/seed_tools/utils/study_validation.test.ts @@ -30,7 +30,7 @@ describe('getStudyErrors', () => { filter: { locale: ['en'], channel: ['CANARY'], - platform: ['WINDOWS'], + platform: ['PLATFORM_WINDOWS'], start_date: Math.floor(new Date('2022-01-01').getTime() / 1000), end_date: Math.floor(new Date('2022-02-01').getTime() / 1000), min_version: '1.0', @@ -141,7 +141,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['LINUX'], + platform: ['PLATFORM_LINUX'], channel: ['BETA'], }, }); @@ -263,7 +263,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['LINUX'], + platform: ['PLATFORM_LINUX'], channel: ['BETA'], }, }); @@ -286,7 +286,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['LINUX'], + platform: ['PLATFORM_LINUX'], channel: ['BETA'], }, }); @@ -362,7 +362,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['LINUX'], + platform: ['PLATFORM_LINUX'], channel: ['BETA'], }, }; @@ -543,7 +543,7 @@ describe('getStudyErrors', () => { }, ], filter: { - platform: ['LINUX'], + platform: ['PLATFORM_LINUX'], channel: ['BETA'], locale: [], exclude_locale: ['en'], @@ -634,7 +634,7 @@ describe('getStudyErrors', () => { probability_weight: 100, }, ], - filter: { ...filter, platform: ['LINUX'], channel: ['BETA'] }, + filter: { ...filter, platform: ['PLATFORM_LINUX'], channel: ['BETA'] }, }); expect(study_validation.getStudyErrors(study, studyFileBaseName)).toEqual( @@ -693,7 +693,7 @@ describe('getStudyErrors', () => { probability_weight: 100, }, ], - filter: { ...filter, platform: ['LINUX'] }, + filter: { ...filter, platform: ['PLATFORM_LINUX'] }, }); expect( @@ -702,23 +702,24 @@ describe('getStudyErrors', () => { }, ); - test.each([{}, { platform: [] }, { platform: ['LINUX', 'LINUX'] }])( - 'should error if platform is invalid', - (filter: any) => { - const study = Study.fromJson({ - name: 'study', - experiment: [ - { - name: 'experiment1', - probability_weight: 100, - }, - ], - filter: { ...filter, channel: ['BETA'] }, - }); + test.each([ + {}, + { platform: [] }, + { platform: ['PLATFORM_LINUX', 'PLATFORM_LINUX'] }, + ])('should error if platform is invalid', (filter: any) => { + const study = Study.fromJson({ + name: 'study', + experiment: [ + { + name: 'experiment1', + probability_weight: 100, + }, + ], + filter: { ...filter, channel: ['BETA'] }, + }); - expect( - study_validation.getStudyErrors(study, studyFileBaseName), - ).toContainEqual(expect.stringMatching(/(P|p)latform/)); - }, - ); + expect( + study_validation.getStudyErrors(study, studyFileBaseName), + ).toContainEqual(expect.stringMatching(/(P|p)latform/)); + }); }); From 9706642b2825869dffe7c69c6c07f7997855ae09 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:42:56 +0400 Subject: [PATCH 05/10] Fix tests --- src/test/data/seed1.bin.processing_expectations | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/data/seed1.bin.processing_expectations b/src/test/data/seed1.bin.processing_expectations index 46a591ec..4a861f33 100644 --- a/src/test/data/seed1.bin.processing_expectations +++ b/src/test/data/seed1.bin.processing_expectations @@ -54,9 +54,9 @@ } ], "filter": { + "end_date": "Wed, 14 Sep 2022 17:10:24 GMT", "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID", - "end_date": "Wed, 14 Sep 2022 17:10:24 GMT" + "platform": "WINDOWS, MAC, LINUX, ANDROID" } } ], From 7df5791d5cafbab9e318ddcb3a7572268630270d Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:45:05 +0400 Subject: [PATCH 06/10] Remove protobufjs --- package.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/package.json b/package.json index 0ae75a6e..f3951a2c 100644 --- a/package.json +++ b/package.json @@ -38,8 +38,6 @@ "prettier": "3.3.3", "prettier-plugin-multiline-arrays": "3.0.6", "prettier-plugin-organize-imports": "3.2.4", - "protobufjs": "7.4.0", - "protobufjs-cli": "1.1.3", "style-loader": "3.3.4", "ts-jest": "29.2.5", "ts-loader": "9.5.1", From b95bfc20b2b1779630f5c478ed34e651cc3efe66 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 17:49:10 +0400 Subject: [PATCH 07/10] Fix proto gen --- src/scripts/generate_proto_ts.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/scripts/generate_proto_ts.ts b/src/scripts/generate_proto_ts.ts index 9d8408ad..de01ba7f 100644 --- a/src/scripts/generate_proto_ts.ts +++ b/src/scripts/generate_proto_ts.ts @@ -74,7 +74,7 @@ function generateProtobufTs() { function generateStudyProtoPatch() { fs.writeFileSync( - `${protoDir}/study.protobuf-ts.patch`, + `${protoDir}/study.proto.protobuf-ts.patch`, execSync(`git diff ${protoDir}/study.proto`, { encoding: 'buffer', }), @@ -82,14 +82,14 @@ function generateStudyProtoPatch() { } function gitApplyStudyProtoPatch() { - execSync(`git apply ${protoDir}/study.protobuf-ts.patch`, { + execSync(`git apply ${protoDir}/study.proto.protobuf-ts.patch`, { cwd: protoDir, stdio: 'inherit', }); } function gitRevertStudyProtoPatch() { - execSync(`git apply -R ${protoDir}/study.protobuf-ts.patch`, { + execSync(`git apply -R ${protoDir}/study.proto.protobuf-ts.patch`, { cwd: protoDir, stdio: 'inherit', }); From 93138ec2e9fa0917ba83e15b46d5171c1dec9c51 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 18:49:23 +0400 Subject: [PATCH 08/10] Use seed_tool serialization --- src/core/serializers.ts | 41 +- src/finch_tracker/main.ts | 2 +- src/finch_tracker/tracker_lib.test.ts | 36 +- src/finch_tracker/tracker_lib.ts | 27 +- .../data/seed1.bin.processing_expectations | 694 +++++++++++------- 5 files changed, 455 insertions(+), 345 deletions(-) diff --git a/src/core/serializers.ts b/src/core/serializers.ts index 50f91001..4da2cb09 100644 --- a/src/core/serializers.ts +++ b/src/core/serializers.ts @@ -2,7 +2,7 @@ // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this file, // You can obtain one at https://mozilla.org/MPL/2.0/. -import { Study, Study_Channel, Study_Platform } from '../proto/generated/study'; +import { Study_Channel, Study_Platform } from '../proto/generated/study'; export function getPlatformNameFromString(protoPlatfrom: string): string { const PREFIX = 'PLATFORM_'; @@ -32,42 +32,3 @@ export function getChannelName( ): string { return getChannelNameFromString(Study_Channel[protoChannel], isBraveSpecific); } - -function unixSecondToUTCString(unixTimeSeconds: number): string { - return new Date(unixTimeSeconds * 1000).toUTCString(); -} - -export function serializePlatforms(platforms?: string[]): string | undefined { - if (platforms === undefined) return undefined; - return platforms.map((v) => getPlatformNameFromString(v)).join(', '); -} - -export function serializeChannels(channels?: string[]): string | undefined { - if (channels === undefined) return undefined; - return channels.join(', '); -} - -// Converts a study to JSON that is ready to be serialized. Some field are -// removed, some are converted to a human readable format. -export function studyToJSON(study: Study): Record { - const json = Study.toJson(study, { useProtoFieldName: true }) as Record< - string, - any - > | null; - if (json === null) { - throw new Error('Failed to convert study to JSON'); - } - const filter = json.filter; - delete json.consistency; - delete json.activation_type; - if (filter !== undefined) { - if (filter.end_date !== undefined) - filter.end_date = unixSecondToUTCString(filter.end_date); - if (filter.start_date !== undefined) { - filter.start_date = unixSecondToUTCString(filter.start_date); - } - filter.platform = serializePlatforms(filter.platform); - filter.channel = serializeChannels(filter.channel); - } - return json; -} diff --git a/src/finch_tracker/main.ts b/src/finch_tracker/main.ts index 27bd543c..b3b9647b 100644 --- a/src/finch_tracker/main.ts +++ b/src/finch_tracker/main.ts @@ -98,7 +98,7 @@ async function main(): Promise { } if (updateData) { - storeDataToDirectory(seedData, storageDir, options); + await storeDataToDirectory(seedData, storageDir, options); if (commitData) { newGitSha1 = commitAllChanges(storageDir); } diff --git a/src/finch_tracker/tracker_lib.test.ts b/src/finch_tracker/tracker_lib.test.ts index 8f57c577..7967fdb6 100644 --- a/src/finch_tracker/tracker_lib.test.ts +++ b/src/finch_tracker/tracker_lib.test.ts @@ -4,32 +4,42 @@ // You can obtain one at https://mozilla.org/MPL/2.0/. import * as fs from 'fs'; +import * as os from 'os'; import { describe, expect, test } from '@jest/globals'; +import path from 'path'; import { StudyPriority } from '../core/study_processor'; import { ItemAction, makeSummary, summaryToJson } from '../core/summary'; import { Study, Study_Channel, Study_Platform } from '../proto/generated/study'; import { VariationsSeed } from '../proto/generated/variations_seed'; -import { serializeStudies } from './tracker_lib'; - -function serialize(json: Record) { - const ordered = Object.keys(json) - .sort() - .reduce((res: Record, key) => { - res[key] = json[key]; - return res; - }, {}); - return JSON.stringify(ordered, undefined, 2); +import { storeDataToDirectory } from './tracker_lib'; + +function readDirectory(dir: string): string { + const files = fs + .readdirSync(dir, { recursive: true, encoding: 'utf-8' }) + .sort(); + let result = ''; + + for (const file of files) { + const filePath = path.join(dir, file); + if (!file.endsWith('.json5')) { + continue; + } + const content = fs.readFileSync(filePath, 'utf-8'); + result += file + '\n' + content + '\n'; + } + return result; } -test('seed serialization', () => { +test('seed serialization', async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tracker-')); const data = fs.readFileSync('src/test/data/seed1.bin'); - const map = serializeStudies(data, { + await storeDataToDirectory(data, tempDir, { minMajorVersion: 116, isBraveSeed: true, }); - const serializedOutput = serialize(map); + const serializedOutput = readDirectory(path.join(tempDir)); const serializedExpectations = fs .readFileSync('src/test/data/seed1.bin.processing_expectations') .toString(); diff --git a/src/finch_tracker/tracker_lib.ts b/src/finch_tracker/tracker_lib.ts index 7c1149a6..6a752ebb 100644 --- a/src/finch_tracker/tracker_lib.ts +++ b/src/finch_tracker/tracker_lib.ts @@ -8,7 +8,6 @@ import * as fs from 'fs'; import * as path from 'path'; import { type ProcessingOptions } from '../core/base_types'; -import { studyToJSON } from '../core/serializers'; import { ProcessedStudy, StudyPriority, @@ -16,6 +15,7 @@ import { } from '../core/study_processor'; import { Study } from '../proto/generated/study'; import { VariationsSeed } from '../proto/generated/variations_seed'; +import { writeStudyFile } from '../seed_tools/utils/study_json_utils'; import { downloadUrl, getSeedPath, getStudyPath } from './node_utils'; export async function fetchChromeSeedData(): Promise { @@ -24,18 +24,17 @@ export async function fetchChromeSeedData(): Promise { return await downloadUrl(kChromeSeedUrl); } -// Processes, groups by name and converts to JSON a list of studies. -export function serializeStudies( +// Groups studies by name and priority. +export function groupStudies( seedData: Buffer, options: ProcessingOptions, -): Record { - const map: Record = {}; +): Record { + const map: Record = {}; const seed = VariationsSeed.fromBinary(seedData); const addStudy = (path: string, study: Study) => { - const json = studyToJSON(study); const list = map[path]; - if (list !== undefined) list.push(json); - else map[path] = [json]; + if (list !== undefined) list.push(study); + else map[path] = [study]; }; for (const study of seed.study) { @@ -74,20 +73,20 @@ export function commitAllChanges(directory: string): string | undefined { // Processes and serializes a given seed to disk (including grouping to // subdirectories/files). -export function storeDataToDirectory( +export async function storeDataToDirectory( seedData: Buffer, directory: string, options: ProcessingOptions, -): void { +): Promise { const studyDirectory = getStudyPath(directory); fs.rmSync(studyDirectory, { recursive: true, force: true }); - const map = serializeStudies(seedData, options); + const map = groupStudies(seedData, options); - for (const [name, json] of Object.entries(map)) { - const fileName = `${studyDirectory}/${name}`; + for (const [name, study] of Object.entries(map)) { + const fileName = `${studyDirectory}/${name}.json5`; const dirname = path.dirname(fileName); fs.mkdirSync(dirname, { recursive: true }); - fs.writeFileSync(fileName, JSON.stringify(json, null, 2) + '\n'); + await writeStudyFile(study, fileName, { isChromium: !options.isBraveSeed }); } // TODO: maybe start to use s3 instead of git one day? diff --git a/src/test/data/seed1.bin.processing_expectations b/src/test/data/seed1.bin.processing_expectations index 4a861f33..136a186b 100644 --- a/src/test/data/seed1.bin.processing_expectations +++ b/src/test/data/seed1.bin.processing_expectations @@ -1,298 +1,438 @@ -{ - "all-by-name/BetaStudy": [ - { - "name": "BetaStudy", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } - } +study/all-by-name/BetaStudy.json5 +[ + { + name: 'BetaStudy', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], + }, + }, + ], + filter: { + channel: [ + 'BETA', + ], + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', ], - "filter": { - "channel": "BETA", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/BlocklistedStudy": [ - { - "name": "BlocklistedStudy", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "Ukm" - ] - } - } + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/BlocklistedStudy.json5 +[ + { + name: 'BlocklistedStudy', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'Ukm', + ], + }, + }, + ], + filter: { + channel: [ + 'RELEASE', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/EndedByDate": [ - { - "name": "EndedByDate", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/EndedByDate.json5 +[ + { + name: 'EndedByDate', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], + }, + }, + ], + filter: { + end_date: '2022-09-14T17:10:24.000Z', + channel: [ + 'RELEASE', ], - "filter": { - "end_date": "Wed, 14 Sep 2022 17:10:24 GMT", - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/EndedMaxVersion": [ - { - "name": "EndedMaxVersion", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', ], - "filter": { - "max_version": "99.1.49.83", - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/Stable-100": [ - { - "name": "Stable-100", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 99, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/EndedMaxVersion.json5 +[ + { + name: 'EndedMaxVersion', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Default", - "probability_weight": 1 - } + }, + ], + filter: { + max_version: '99.1.49.83', + channel: [ + 'RELEASE', + ], + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/Stable-100.json5 +[ + { + name: 'Stable-100', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 99, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], + }, + }, + { + name: 'Default', + probability_weight: 1, + }, + ], + filter: { + channel: [ + 'RELEASE', + ], + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/Stable-50": [ - { - "name": "Stable-50", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 50, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/Stable-50.json5 +[ + { + name: 'Stable-50', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 50, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Default", - "probability_weight": 50 - } + }, + { + name: 'Default', + probability_weight: 50, + }, + ], + filter: { + channel: [ + 'RELEASE', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/Stable-min": [ - { - "name": "Stable-min", - "experiment": [ - { - "name": "Enabled_1", - "probability_weight": 5, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/Stable-min.json5 +[ + { + name: 'Stable-min', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled_1', + probability_weight: 5, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Control_1", - "probability_weight": 45 + }, + { + name: 'Control_1', + probability_weight: 45, + }, + { + name: 'Default', + probability_weight: 50, + }, + ], + filter: { + channel: [ + 'RELEASE', + ], + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/all-by-name/StudyKillSwitch.json5 +[ + { + name: 'StudyKillSwitch', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Default", - "probability_weight": 50 - } + }, + ], + filter: { + max_version: '299.0.0.0', + channel: [ + 'RELEASE', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "all-by-name/StudyKillSwitch": [ - { - "name": "StudyKillSwitch", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/blocklisted/BlocklistedStudy.json5 +[ + { + name: 'BlocklistedStudy', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'Ukm', + ], + }, + }, + ], + filter: { + channel: [ + 'RELEASE', ], - "filter": { - "max_version": "299.0.0.0", - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "blocklisted/BlocklistedStudy": [ - { - "name": "BlocklistedStudy", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "Ukm" - ] - } - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "stable-100%/Stable-100": [ - { - "name": "Stable-100", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 99, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/stable-100%/Stable-100.json5 +[ + { + name: 'Stable-100', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 99, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Default", - "probability_weight": 1 - } + }, + { + name: 'Default', + probability_weight: 1, + }, + ], + filter: { + channel: [ + 'RELEASE', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "stable-50%/Stable-50": [ - { - "name": "Stable-50", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 50, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/stable-50%/Stable-50.json5 +[ + { + name: 'Stable-50', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 50, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Default", - "probability_weight": 50 - } + }, + { + name: 'Default', + probability_weight: 50, + }, + ], + filter: { + channel: [ + 'RELEASE', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "stable-emergency-kill-switch/StudyKillSwitch": [ - { - "name": "StudyKillSwitch", - "experiment": [ - { - "name": "Enabled", - "probability_weight": 100, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } - } + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', ], - "filter": { - "max_version": "299.0.0.0", - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ], - "stable-min/Stable-min": [ - { - "name": "Stable-min", - "experiment": [ - { - "name": "Enabled_1", - "probability_weight": 5, - "feature_association": { - "enable_feature": [ - "SomeFeature" - ] - } + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/stable-emergency-kill-switch/StudyKillSwitch.json5 +[ + { + name: 'StudyKillSwitch', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled', + probability_weight: 100, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Control_1", - "probability_weight": 45 + }, + ], + filter: { + max_version: '299.0.0.0', + channel: [ + 'RELEASE', + ], + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', + ], + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] + +study/stable-min/Stable-min.json5 +[ + { + name: 'Stable-min', + consistency: 'PERMANENT', + experiment: [ + { + name: 'Enabled_1', + probability_weight: 5, + feature_association: { + enable_feature: [ + 'SomeFeature', + ], }, - { - "name": "Default", - "probability_weight": 50 - } + }, + { + name: 'Control_1', + probability_weight: 45, + }, + { + name: 'Default', + probability_weight: 50, + }, + ], + filter: { + channel: [ + 'RELEASE', + ], + platform: [ + 'WINDOWS', + 'MAC', + 'LINUX', + 'ANDROID', ], - "filter": { - "channel": "STABLE", - "platform": "WINDOWS, MAC, LINUX, ANDROID" - } - } - ] -} \ No newline at end of file + }, + activation_type: 'ACTIVATE_ON_STARTUP', + }, +] From f1e2cecd7e61c572011696d5657affeea49267e6 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 18:55:19 +0400 Subject: [PATCH 09/10] Adjust URLs --- src/core/summary.ts | 2 +- src/core/url_utils.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/summary.ts b/src/core/summary.ts index a2ee2acc..82266ff7 100644 --- a/src/core/summary.ts +++ b/src/core/summary.ts @@ -215,7 +215,7 @@ function getGitHubDiffUrl( oldPriority: StudyPriority, commit: string, ): string { - const path = `study/all-by-name/${study}`; + const path = `study/all-by-name/${study}.json5`; const pathHash = sha256(path); return `${url_utils.getGitHubStorageUrl()}/commit/${commit}#diff-${pathHash}`; } diff --git a/src/core/url_utils.ts b/src/core/url_utils.ts index 101c1775..4f838a84 100644 --- a/src/core/url_utils.ts +++ b/src/core/url_utils.ts @@ -51,8 +51,7 @@ export function getStudyRawConfigUrl( return ( 'https://github.com/search?type=code' + '&q=repo%3Abrave%2Fbrave-variations' + - '+path%3A%2F%5Eseed%5C%2Fseed.json%7C%5Estudies%5C%2F*.json5%2F+' + - `"%5C"name%5C"%3A+%5C"${encodeURIComponent(study)}%5C""` + `+path%3Astudies%2F+%22name%3A+%27${encodeURIComponent(study)}%27%22` ); } From 3760af1c1f06dbf3ed4c0dfc52c662714199b30e Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Wed, 6 Nov 2024 19:04:48 +0400 Subject: [PATCH 10/10] Fix tests --- src/finch_tracker/tracker_lib.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/finch_tracker/tracker_lib.test.ts b/src/finch_tracker/tracker_lib.test.ts index 7967fdb6..9b7eeb9a 100644 --- a/src/finch_tracker/tracker_lib.test.ts +++ b/src/finch_tracker/tracker_lib.test.ts @@ -26,7 +26,8 @@ function readDirectory(dir: string): string { continue; } const content = fs.readFileSync(filePath, 'utf-8'); - result += file + '\n' + content + '\n'; + if (result != '') result += '\n'; + result += file + '\n' + content; } return result; }