More tests for bucket id transformer

Author: simolus3

packages/sync-rules/src/BaseSqlDataQuery.ts

@@ -177,9 +177,9 @@ export class BaseSqlDataQuery {
     }
   }
 
-  evaluateRowWithOptions(options: EvaluateRowOptions): EvaluationResult[] {
+  evaluateRowWithOptions(options: Omit<EvaluateRowOptions, 'bucketIdTransformer'>): EvaluationResult[] {
     try {
-      const { table, row, bucketIds, bucketIdTransformer } = options;
+      const { table, row, bucketIds } = options;
 
       const tables = { [this.table]: this.addSpecialParameters(table, row) };
       const resolvedBucketIds = bucketIds(tables);
@@ -199,7 +199,7 @@ export class BaseSqlDataQuery {
 
       return resolvedBucketIds.map((bucketId) => {
         return {
-          bucket: bucketIdTransformer ? bucketIdTransformer(bucketId) : bucketId,
+          bucket: bucketId,
           table: outputTable,
           id: id,
           data

packages/sync-rules/src/SqlBucketDescriptor.ts

@@ -13,6 +13,7 @@ import { TableValuedFunctionSqlParameterQuery } from './TableValuedFunctionSqlPa
 import { SqlRuleError } from './errors.js';
 import { CompatibilityContext } from './compatibility.js';
 import {
+  BucketIdTransformer,
   EvaluatedParametersResult,
   EvaluateRowOptions,
   EvaluationResult,
@@ -137,7 +138,11 @@ export class SqlBucketDescriptor implements BucketSource {
 
   pushBucketParameterQueriers(result: PendingQueriers, options: GetQuerierOptions) {
     const reasons = [this.bucketInclusionReason()];
-    const staticBuckets = this.getStaticBucketDescriptions(options.globalParameters, reasons);
+    const staticBuckets = this.getStaticBucketDescriptions(
+      options.globalParameters,
+      reasons,
+      options.bucketIdTransformer
+    );
     const staticQuerier = {
       staticBuckets,
       hasDynamicBuckets: false,
@@ -151,15 +156,19 @@ export class SqlBucketDescriptor implements BucketSource {
     }
 
     const dynamicQueriers = this.parameterQueries.map((query) =>
-      query.getBucketParameterQuerier(options.globalParameters, reasons)
+      query.getBucketParameterQuerier(options.globalParameters, reasons, options.bucketIdTransformer)
     );
     result.queriers.push(...dynamicQueriers);
   }
 
-  getStaticBucketDescriptions(parameters: RequestParameters, reasons: BucketInclusionReason[]): ResolvedBucket[] {
+  getStaticBucketDescriptions(
+    parameters: RequestParameters,
+    reasons: BucketInclusionReason[],
+    transformer: BucketIdTransformer
+  ): ResolvedBucket[] {
     let results: ResolvedBucket[] = [];
     for (let query of this.globalParameterQueries) {
-      for (const desc of query.getStaticBucketDescriptions(parameters)) {
+      for (const desc of query.getStaticBucketDescriptions(parameters, transformer)) {
         results.push({
           ...desc,
           definition: this.name,

packages/sync-rules/src/SqlDataQuery.ts

@@ -188,15 +188,16 @@ export class SqlDataQuery extends BaseSqlDataQuery {
   evaluateRow(
     table: SourceTableInterface,
     row: SqliteRow,
-    bucketIdTransformer: BucketIdTransformer | null
+    bucketIdTransformer: BucketIdTransformer
   ): EvaluationResult[] {
     return this.evaluateRowWithOptions({
       table,
       row,
-      bucketIdTransformer,
       bucketIds: (tables) => {
         const bucketParameters = this.filter.filterRow(tables);
-        return bucketParameters.map((params) => getBucketId(this.descriptorName, this.bucketParameters, params));
+        return bucketParameters.map((params) =>
+          getBucketId(this.descriptorName, this.bucketParameters, params, bucketIdTransformer)
+        );
       }
     });
   }

packages/sync-rules/src/SqlParameterQuery.ts

@@ -15,6 +15,7 @@ import { TablePattern } from './TablePattern.js';
 import { TableQuerySchema } from './TableQuerySchema.js';
 import { TableValuedFunctionSqlParameterQuery } from './TableValuedFunctionSqlParameterQuery.js';
 import {
+  BucketIdTransformer,
   EvaluatedParameters,
   EvaluatedParametersResult,
   InputParameter,
@@ -348,7 +349,11 @@ export class SqlParameterQuery {
    *
    * Internal function, but exposed for tests.
    */
-  resolveBucketDescriptions(bucketParameters: SqliteJsonRow[], parameters: RequestParameters): BucketDescription[] {
+  resolveBucketDescriptions(
+    bucketParameters: SqliteJsonRow[],
+    parameters: RequestParameters,
+    transformer: BucketIdTransformer
+  ): BucketDescription[] {
     // Filters have already been applied and gotten us the set of bucketParameters - don't attempt to filter again.
     // We _do_ need to evaluate the output columns here, using a combination of precomputed bucketParameters,
     // and values from token parameters.
@@ -372,7 +377,7 @@ export class SqlParameterQuery {
         }
 
         return {
-          bucket: getBucketId(this.descriptorName, this.bucketParameters, result),
+          bucket: getBucketId(this.descriptorName, this.bucketParameters, result, transformer),
           priority: this.priority
         };
       })
@@ -457,7 +462,8 @@ export class SqlParameterQuery {
 
   getBucketParameterQuerier(
     requestParameters: RequestParameters,
-    reasons: BucketInclusionReason[]
+    reasons: BucketInclusionReason[],
+    transformer: BucketIdTransformer
   ): BucketParameterQuerier {
     const lookups = this.getLookups(requestParameters);
     if (lookups.length == 0) {
@@ -477,7 +483,7 @@ export class SqlParameterQuery {
       parameterQueryLookups: lookups,
       queryDynamicBucketDescriptions: async (source: ParameterLookupSource) => {
         const bucketParameters = await source.getParameterSets(lookups);
-        return this.resolveBucketDescriptions(bucketParameters, requestParameters).map((bucket) => ({
+        return this.resolveBucketDescriptions(bucketParameters, requestParameters, transformer).map((bucket) => ({
           ...bucket,
           definition: this.descriptorName,
           inclusion_reasons: reasons

packages/sync-rules/src/SqlSyncRules.ts

@@ -399,7 +399,7 @@ export class SqlSyncRules implements SyncRules {
       : {
           ...options,
           // Disable bucket id transformer when the option is unused.
-          transformBucketIds: null
+          bucketIdTransformer: (id: string) => id
         };
 
     let rawResults: EvaluationResult[] = [];
@@ -439,13 +439,24 @@ export class SqlSyncRules implements SyncRules {
   }
 
   getBucketParameterQuerier(options: GetQuerierOptions): GetBucketParameterQuerierResult {
+    const resolvedOptions = this.compatibility.isEnabled(CompatibilityOption.versionedBucketIds)
+      ? options
+      : {
+          ...options,
+          // Disable bucket id transformer when the option is unused.
+          bucketIdTransformer: (id: string) => id
+        };
+
     const queriers: BucketParameterQuerier[] = [];
     const errors: QuerierError[] = [];
     const pending = { queriers, errors };
 
     for (const source of this.bucketSources) {
-      if ((source.subscribedToByDefault && options.hasDefaultStreams) || source.name in options.streams) {
-        source.pushBucketParameterQueriers(pending, options);
+      if (
+        (source.subscribedToByDefault && resolvedOptions.hasDefaultStreams) ||
+        source.name in resolvedOptions.streams
+      ) {
+        source.pushBucketParameterQueriers(pending, resolvedOptions);
       }
     }
 

packages/sync-rules/src/StaticSqlParameterQuery.ts

@@ -3,7 +3,13 @@ import { BucketDescription, BucketPriority, DEFAULT_BUCKET_PRIORITY } from './Bu
 import { SqlRuleError } from './errors.js';
 import { SqlTools } from './sql_filters.js';
 import { checkUnsupportedFeatures, isClauseError, isParameterValueClause, sqliteBool } from './sql_support.js';
-import { ParameterValueClause, QueryParseOptions, RequestParameters, SqliteJsonValue } from './types.js';
+import {
+  BucketIdTransformer,
+  ParameterValueClause,
+  QueryParseOptions,
+  RequestParameters,
+  SqliteJsonValue
+} from './types.js';
 import { getBucketId, isJsonValue } from './utils.js';
 
 export interface StaticSqlParameterQueryOptions {
@@ -153,7 +159,7 @@ export class StaticSqlParameterQuery {
     this.errors = options.errors ?? [];
   }
 
-  getStaticBucketDescriptions(parameters: RequestParameters): BucketDescription[] {
+  getStaticBucketDescriptions(parameters: RequestParameters, transformer: BucketIdTransformer): BucketDescription[] {
     if (this.filter == null) {
       // Error in filter clause
       return [];
@@ -177,7 +183,7 @@ export class StaticSqlParameterQuery {
 
     return [
       {
-        bucket: getBucketId(this.descriptorName, this.bucketParameters, result),
+        bucket: getBucketId(this.descriptorName, this.bucketParameters, result, transformer),
         priority: this.priority
       }
     ];

packages/sync-rules/src/TableValuedFunctionSqlParameterQuery.ts

@@ -4,6 +4,7 @@ import { SqlTools } from './sql_filters.js';
 import { checkUnsupportedFeatures, isClauseError, isParameterValueClause, sqliteBool } from './sql_support.js';
 import { TABLE_VALUED_FUNCTIONS, TableValuedFunction } from './TableValuedFunctions.js';
 import {
+  BucketIdTransformer,
   ParameterValueClause,
   ParameterValueSet,
   QueryParseOptions,
@@ -202,7 +203,7 @@ export class TableValuedFunctionSqlParameterQuery {
     this.errors = options.errors;
   }
 
-  getStaticBucketDescriptions(parameters: RequestParameters): BucketDescription[] {
+  getStaticBucketDescriptions(parameters: RequestParameters, transformer: BucketIdTransformer): BucketDescription[] {
     if (this.filter == null || this.callClause == null) {
       // Error in filter clause
       return [];
@@ -212,15 +213,19 @@ export class TableValuedFunctionSqlParameterQuery {
     const rows = this.function.call([valueString]);
     let total: BucketDescription[] = [];
     for (let row of rows) {
-      const description = this.getIndividualBucketDescription(row, parameters);
+      const description = this.getIndividualBucketDescription(row, parameters, transformer);
       if (description !== null) {
         total.push(description);
       }
     }
     return total;
   }
 
-  private getIndividualBucketDescription(row: SqliteRow, parameters: RequestParameters): BucketDescription | null {
+  private getIndividualBucketDescription(
+    row: SqliteRow,
+    parameters: RequestParameters,
+    transformer: BucketIdTransformer
+  ): BucketDescription | null {
     const mergedParams: ParameterValueSet = {
       ...parameters,
       lookup: (table, column) => {
@@ -247,7 +252,7 @@ export class TableValuedFunctionSqlParameterQuery {
     }
 
     return {
-      bucket: getBucketId(this.descriptorName, this.bucketParameters, result),
+      bucket: getBucketId(this.descriptorName, this.bucketParameters, result, transformer),
       priority: this.priority
     };
   }

packages/sync-rules/src/streams/stream.ts

@@ -181,11 +181,10 @@ export class SyncStream implements BucketSource {
     return this.data.evaluateRowWithOptions({
       table: options.sourceTable,
       row: applyRowContext(options.record, this.compatibility),
-      bucketIdTransformer: options.bucketIdTransformer,
       bucketIds() {
         const bucketIds: string[] = [];
         for (const variant of stream.variants) {
-          bucketIds.push(...variant.bucketIdsForRow(stream.name, row));
+          bucketIds.push(...variant.bucketIdsForRow(stream.name, row, options.bucketIdTransformer));
         }
 
         return bucketIds;

packages/sync-rules/src/streams/variant.ts

@@ -68,8 +68,8 @@ export class StreamVariant {
   /**
    * Given a row in the table this stream selects from, returns all ids of buckets to which that row belongs to.
    */
-  bucketIdsForRow(streamName: string, options: TableRow): string[] {
-    return this.instantiationsForRow(options).map((values) => this.buildBucketId(streamName, values));
+  bucketIdsForRow(streamName: string, options: TableRow, transformer: BucketIdTransformer): string[] {
+    return this.instantiationsForRow(options).map((values) => this.buildBucketId(streamName, values, transformer));
   }
 
   /**
@@ -305,14 +305,15 @@ export class StreamVariant {
    *
    * @param streamName The name of the stream, included in the bucket id
    * @param instantiation An instantiation for all parameters in this variant.
+   * @param transformer A transformer adding version information to the inner id.
    * @returns The generated bucket id
    */
-  private buildBucketId(streamName: string, instantiation: SqliteJsonValue[]) {
+  private buildBucketId(streamName: string, instantiation: SqliteJsonValue[], transformer: BucketIdTransformer) {
     if (instantiation.length != this.parameters.length) {
       throw Error('Internal error, instantiation length mismatch');
     }
 
-    return `${streamName}|${this.id}${JSONBucketNameSerialize.stringify(instantiation)}`;
+    return transformer(`${streamName}|${this.id}${JSONBucketNameSerialize.stringify(instantiation)}`);
   }
 
   private resolveBucket(
@@ -324,7 +325,7 @@ export class StreamVariant {
     return {
       definition: stream.name,
       inclusion_reasons: [reason],
-      bucket: bucketIdTransformer(this.buildBucketId(stream.name, instantiation)),
+      bucket: this.buildBucketId(stream.name, instantiation, bucketIdTransformer),
       priority: stream.priority
     };
   }

packages/sync-rules/src/types.ts

@@ -298,7 +298,7 @@ export interface InputParameter {
 export type BucketIdTransformer = (regularId: string) => string;
 
 export interface EvaluateRowOptions extends TableRow<SqliteInputRow> {
-  bucketIdTransformer: BucketIdTransformer | null;
+  bucketIdTransformer: BucketIdTransformer;
 }
 
 /**