Skip to content

Commit

Permalink
Avoid parsing feature-set defaults at module initialization (#683)
Browse files Browse the repository at this point in the history
  • Loading branch information
timostamm authored Jan 29, 2024
1 parent db0bd03 commit 175bc97
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 20 deletions.
15 changes: 10 additions & 5 deletions packages/protobuf/src/create-descriptor-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@ import {
parseTextFormatEnumValue,
parseTextFormatScalarValue,
} from "./private/text-format.js";
import type { BinaryReadOptions, BinaryWriteOptions } from "./binary-format.js";
import type { FeatureResolverFn } from "./private/feature-set.js";
import {
createFeatureResolver,
featureSetDefaults,
} from "./private/feature-set.js";
import { createFeatureResolver } from "./private/feature-set.js";

/**
* Create a DescriptorSet, a convenient interface for working with a set of
Expand Down Expand Up @@ -90,8 +88,9 @@ export function createDescriptorSet(
let resolveFeatures = resolverByEdition.get(edition);
if (resolveFeatures === undefined) {
resolveFeatures = createFeatureResolver(
options?.featureSetDefaults ?? featureSetDefaults,
edition,
options?.featureSetDefaults,
options?.serializationOptions,
);
resolverByEdition.set(edition, resolveFeatures);
}
Expand All @@ -117,6 +116,12 @@ interface CreateDescriptorSetOptions {
* `--experimental_edition_defaults_out`.
*/
featureSetDefaults?: FeatureSetDefaults;

/**
* Internally, data is serialized when features are resolved. The
* serialization options given here will be used for feature resolution.
*/
serializationOptions?: Partial<BinaryReadOptions & BinaryWriteOptions>;
}

/**
Expand Down
43 changes: 28 additions & 15 deletions packages/protobuf/src/private/feature-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,24 @@ import {
FeatureSetDefaults,
} from "../google/protobuf/descriptor_pb.js";
import { protoBase64 } from "../proto-base64.js";
import type {
BinaryReadOptions,
BinaryWriteOptions,
} from "../binary-format.js";

/**
* Static edition feature defaults supported by @bufbuild/protobuf.
* Return the edition feature defaults supported by @bufbuild/protobuf.
*/
export const featureSetDefaults = FeatureSetDefaults.fromBinary(
protoBase64.dec(
/*upstream-inject-feature-defaults-start*/ "ChESDAgBEAIYAiABKAEwAhjmBwoREgwIAhABGAEgAigBMAEY5wcKERIMCAEQARgBIAIoATABGOgHIOYHKOgH" /*upstream-inject-feature-defaults-end*/,
),
);
function getFeatureSetDefaults(
options?: Partial<BinaryReadOptions>,
): FeatureSetDefaults {
return FeatureSetDefaults.fromBinary(
protoBase64.dec(
/*upstream-inject-feature-defaults-start*/ "ChESDAgBEAIYAiABKAEwAhjmBwoREgwIAhABGAEgAigBMAEY5wcKERIMCAEQARgBIAIoATABGOgHIOYHKOgH" /*upstream-inject-feature-defaults-end*/,
),
options,
);
}

/**
* A merged google.protobuf.FeaturesSet, with all fields guaranteed to be set.
Expand All @@ -46,18 +55,22 @@ export type FeatureResolverFn = (
) => MergedFeatureSet;

/**
* Create an edition feature resolver with the given feature set defaults.
* Create an edition feature resolver with the given feature set defaults, or
* the feature set defaults supported by @bufbuild/protobuf.
*/
export function createFeatureResolver(
compiledFeatureSetDefaults: FeatureSetDefaults,
edition: Edition,
compiledFeatureSetDefaults?: FeatureSetDefaults,
serializationOptions?: Partial<BinaryReadOptions & BinaryWriteOptions>,
): FeatureResolverFn {
const min = compiledFeatureSetDefaults.minimumEdition;
const max = compiledFeatureSetDefaults.maximumEdition;
const fds =
compiledFeatureSetDefaults ?? getFeatureSetDefaults(serializationOptions);
const min = fds.minimumEdition;
const max = fds.maximumEdition;
if (
min === undefined ||
max === undefined ||
compiledFeatureSetDefaults.defaults.some((d) => d.edition === undefined)
fds.defaults.some((d) => d.edition === undefined)
) {
throw new Error("Invalid FeatureSetDefaults");
}
Expand All @@ -72,7 +85,7 @@ export function createFeatureResolver(
);
}
let highestMatch: { e: Edition; f: FeatureSet } | undefined = undefined;
for (const c of compiledFeatureSetDefaults.defaults) {
for (const c of fds.defaults) {
const e = c.edition ?? 0;
if (e > edition) {
continue;
Expand All @@ -88,12 +101,12 @@ export function createFeatureResolver(
if (highestMatch === undefined) {
throw new Error(`No valid default found for edition ${Edition[edition]}`);
}
const defaultsBin = highestMatch.f.toBinary();
const featureSetBin = highestMatch.f.toBinary(serializationOptions);
return (...rest): MergedFeatureSet => {
const f = FeatureSet.fromBinary(defaultsBin);
const f = FeatureSet.fromBinary(featureSetBin, serializationOptions);
for (const c of rest) {
if (c !== undefined) {
f.fromBinary(c.toBinary());
f.fromBinary(c.toBinary(serializationOptions), serializationOptions);
}
}
if (!validateMergedFeatures(f)) {
Expand Down

0 comments on commit 175bc97

Please sign in to comment.