Skip to content

refactor(gcpspanner): Migrate WebFeatureChromiumHistogramEnumValue to use Sync #1929

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 14, 2025
Merged

refactor(gcpspanner): Migrate WebFeatureChromiumHistogramEnumValue to use Sync #1929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions lib/gcpspanner/daily_chromium_histogram_metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,8 @@ func TestStoreAndSyncDailyChromiumHistogramMetric(t *testing.T) {
enumIDMap := insertTestChromiumHistogramEnums(ctx, spannerClient, t, sampleEnums)
sampleEnumValues := getSampleChromiumHistogramEnumValues(enumIDMap)
enumValueLabelToIDMap := insertTestChromiumHistogramEnumValues(ctx, spannerClient, t, sampleEnumValues)
spannerClient.createSampleWebFeatureChromiumHistogramEnums(ctx, t, idMap, enumValueLabelToIDMap)
sampleWebFeatureEnumValues := getSampleWebFeatureChromiumHistogramEnums(idMap, enumValueLabelToIDMap)
insertTestWebFeatureChromiumHistogramEnumValues(ctx, spannerClient, t, sampleWebFeatureEnumValues)
sampleMetrics := getSampleDailyChromiumHistogramMetricsToInsert()
insertTestDailyChromiumHistogramMetrics(ctx, spannerClient, t, sampleMetrics)

Expand Down Expand Up @@ -434,7 +435,8 @@ func TestSyncLatestDailyChromiumHistogramMetric_Deletes(t *testing.T) {
enumIDMap := insertTestChromiumHistogramEnums(ctx, spannerClient, t, sampleEnums)
sampleEnumValues := getSampleChromiumHistogramEnumValues(enumIDMap)
enumValueLabelToIDMap := insertTestChromiumHistogramEnumValues(ctx, spannerClient, t, sampleEnumValues)
spannerClient.createSampleWebFeatureChromiumHistogramEnums(ctx, t, idMap, enumValueLabelToIDMap)
sampleWebFeatureEnumValues := getSampleWebFeatureChromiumHistogramEnums(idMap, enumValueLabelToIDMap)
insertTestWebFeatureChromiumHistogramEnumValues(ctx, spannerClient, t, sampleWebFeatureEnumValues)
sampleMetrics := getSampleDailyChromiumHistogramMetricsToInsert()
insertTestDailyChromiumHistogramMetrics(ctx, spannerClient, t, sampleMetrics)

Expand Down
19 changes: 10 additions & 9 deletions lib/gcpspanner/spanneradapters/chromium_historgram_enum_consumer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewChromiumHistogramEnumConsumer(client ChromiumHistogramEnumsClient) *Chro
type ChromiumHistogramEnumsClient interface {
UpsertChromiumHistogramEnum(context.Context, gcpspanner.ChromiumHistogramEnum) (*string, error)
UpsertChromiumHistogramEnumValue(context.Context, gcpspanner.ChromiumHistogramEnumValue) (*string, error)
UpsertWebFeatureChromiumHistogramEnumValue(context.Context, gcpspanner.WebFeatureChromiumHistogramEnumValue) error
SyncWebFeatureChromiumHistogramEnumValues(context.Context, []gcpspanner.WebFeatureChromiumHistogramEnumValue) error
GetIDFromFeatureKey(context.Context, *gcpspanner.FeatureIDFilter) (*string, error)
FetchAllFeatureKeys(context.Context) ([]string, error)
GetAllMovedWebFeatures(ctx context.Context) ([]gcpspanner.MovedWebFeature, error)
Expand Down Expand Up @@ -85,6 +85,7 @@ func (c *ChromiumHistogramEnumConsumer) SaveHistogramEnums(
}

// Create mapping of anticipated enums to feature keys
var mappingsToSync []gcpspanner.WebFeatureChromiumHistogramEnumValue
for histogram, enums := range data {
enumID, err := c.client.UpsertChromiumHistogramEnum(ctx, gcpspanner.ChromiumHistogramEnum{
HistogramName: string(histogram),
Expand Down Expand Up @@ -118,17 +119,17 @@ func (c *ChromiumHistogramEnumConsumer) SaveHistogramEnums(

continue
}
err = c.client.UpsertWebFeatureChromiumHistogramEnumValue(ctx,
gcpspanner.WebFeatureChromiumHistogramEnumValue{
WebFeatureID: *featureID,
ChromiumHistogramEnumValueID: *enumValueID,
})
if err != nil {
return errors.Join(ErrFailedToStoreEnumValueWebFeatureMapping, err)
}
mappingsToSync = append(mappingsToSync, gcpspanner.WebFeatureChromiumHistogramEnumValue{
WebFeatureID: *featureID,
ChromiumHistogramEnumValueID: *enumValueID,
})
}
}

if err := c.client.SyncWebFeatureChromiumHistogramEnumValues(ctx, mappingsToSync); err != nil {
return errors.Join(ErrFailedToStoreEnumValueWebFeatureMapping, err)
}

return nil
}

Expand Down
93 changes: 57 additions & 36 deletions lib/gcpspanner/spanneradapters/chromium_historgram_enum_consumer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ type mockChromiumHistogramEnumsClient struct {
gcpspanner.ChromiumHistogramEnum) (*string, error)
upsertChromiumHistogramEnumValue func(context.Context,
gcpspanner.ChromiumHistogramEnumValue) (*string, error)
upsertWebFeatureChromiumHistogramEnumValue func(context.Context,
gcpspanner.WebFeatureChromiumHistogramEnumValue) error
syncWebFeatureChromiumHistogramEnumValues func(context.Context,
[]gcpspanner.WebFeatureChromiumHistogramEnumValue) error
getIDFromFeatureKey func(context.Context, *gcpspanner.FeatureIDFilter) (*string, error)
fetchAllFeatureKeys func(context.Context) ([]string, error)
getAllMovedWebFeatures func(ctx context.Context) ([]gcpspanner.MovedWebFeature, error)
Expand All @@ -47,9 +47,9 @@ func (m *mockChromiumHistogramEnumsClient) UpsertChromiumHistogramEnumValue(ctx
return m.upsertChromiumHistogramEnumValue(ctx, in)
}

func (m *mockChromiumHistogramEnumsClient) UpsertWebFeatureChromiumHistogramEnumValue(ctx context.Context,
in gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
return m.upsertWebFeatureChromiumHistogramEnumValue(ctx, in)
func (m *mockChromiumHistogramEnumsClient) SyncWebFeatureChromiumHistogramEnumValues(ctx context.Context,
in []gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
return m.syncWebFeatureChromiumHistogramEnumValues(ctx, in)
}

func (m *mockChromiumHistogramEnumsClient) GetIDFromFeatureKey(ctx context.Context,
Expand Down Expand Up @@ -92,11 +92,23 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
_ *gcpspanner.FeatureIDFilter) (*string, error) {
return valuePtr("featureID"), nil
},
upsertWebFeatureChromiumHistogramEnumValue: func(_ context.Context,
_ gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
syncWebFeatureChromiumHistogramEnumValues: func(_ context.Context,
in []gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
expected := []gcpspanner.WebFeatureChromiumHistogramEnumValue{
{
WebFeatureID: "featureID",
ChromiumHistogramEnumValueID: "enumValueID",
},
}
if !reflect.DeepEqual(in, expected) {
t.Errorf("unexpected input to SyncWebFeatureChromiumHistogramEnumValues. got %+v, want %+v",
in, expected)
}

return nil
},
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {

return nil, nil
},
},
Expand All @@ -113,11 +125,11 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
fetchAllFeatureKeys: func(_ context.Context) ([]string, error) {
return nil, errors.New("test error")
},
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
upsertWebFeatureChromiumHistogramEnumValue: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
syncWebFeatureChromiumHistogramEnumValues: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: nil,
},
data: metricdatatypes.HistogramMapping{
metricdatatypes.WebDXFeatureEnum: []metricdatatypes.HistogramEnumValue{
Expand All @@ -136,10 +148,11 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
_ gcpspanner.ChromiumHistogramEnum) (*string, error) {
return nil, errors.New("test error")
},
upsertChromiumHistogramEnumValue: nil,
upsertWebFeatureChromiumHistogramEnumValue: nil,
getIDFromFeatureKey: nil,
upsertChromiumHistogramEnumValue: nil,
syncWebFeatureChromiumHistogramEnumValues: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {

return nil, nil
},
},
Expand All @@ -164,8 +177,8 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
_ gcpspanner.ChromiumHistogramEnumValue) (*string, error) {
return nil, errors.New("test error")
},
upsertWebFeatureChromiumHistogramEnumValue: nil,
getIDFromFeatureKey: nil,
syncWebFeatureChromiumHistogramEnumValues: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {
return nil, nil
},
Expand Down Expand Up @@ -195,8 +208,16 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
_ *gcpspanner.FeatureIDFilter) (*string, error) {
return nil, errors.New("test error")
},
upsertWebFeatureChromiumHistogramEnumValue: nil,
syncWebFeatureChromiumHistogramEnumValues: func(
_ context.Context, in []gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
if len(in) != 0 {
t.Error("expected empty slice")
}

return nil
},
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {

return nil, nil
},
},
Expand All @@ -208,7 +229,7 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
expectedErr: nil,
},
{
name: "UpsertWebFeatureChromiumHistogramEnumValue returns error",
name: "SyncWebFeatureChromiumHistogramEnumValues returns error",
client: &mockChromiumHistogramEnumsClient{
fetchAllFeatureKeys: func(_ context.Context) ([]string, error) {
return []string{"enum-label"}, nil
Expand All @@ -225,8 +246,8 @@ func TestChromiumHistogramEnumConsumer_SaveHistogramEnums(t *testing.T) {
_ *gcpspanner.FeatureIDFilter) (*string, error) {
return valuePtr("featureID"), nil
},
upsertWebFeatureChromiumHistogramEnumValue: func(_ context.Context,
_ gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
syncWebFeatureChromiumHistogramEnumValues: func(_ context.Context,
_ []gcpspanner.WebFeatureChromiumHistogramEnumValue) error {
return errors.New("test error")
},
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {
Expand Down Expand Up @@ -436,11 +457,11 @@ func TestChromiumHistogramEnumConsumer_GetAllMovedWebFeatures(t *testing.T) {
{
name: "Success",
mockClient: &mockChromiumHistogramEnumsClient{
fetchAllFeatureKeys: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
upsertWebFeatureChromiumHistogramEnumValue: nil,
getIDFromFeatureKey: nil,
fetchAllFeatureKeys: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
syncWebFeatureChromiumHistogramEnumValues: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {
return []gcpspanner.MovedWebFeature{
{
Expand Down Expand Up @@ -469,11 +490,11 @@ func TestChromiumHistogramEnumConsumer_GetAllMovedWebFeatures(t *testing.T) {
{
name: "Database error",
mockClient: &mockChromiumHistogramEnumsClient{
fetchAllFeatureKeys: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
upsertWebFeatureChromiumHistogramEnumValue: nil,
getIDFromFeatureKey: nil,
fetchAllFeatureKeys: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
syncWebFeatureChromiumHistogramEnumValues: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {
return nil, errTestDatabaseError
},
Expand All @@ -484,11 +505,11 @@ func TestChromiumHistogramEnumConsumer_GetAllMovedWebFeatures(t *testing.T) {
{
name: "Empty result",
mockClient: &mockChromiumHistogramEnumsClient{
fetchAllFeatureKeys: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
upsertWebFeatureChromiumHistogramEnumValue: nil,
getIDFromFeatureKey: nil,
fetchAllFeatureKeys: nil,
upsertChromiumHistogramEnum: nil,
upsertChromiumHistogramEnumValue: nil,
syncWebFeatureChromiumHistogramEnumValues: nil,
getIDFromFeatureKey: nil,
getAllMovedWebFeatures: func(_ context.Context) ([]gcpspanner.MovedWebFeature, error) {
return []gcpspanner.MovedWebFeature{}, nil
},
Expand Down
60 changes: 38 additions & 22 deletions lib/gcpspanner/web_feature_chromium_histograms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,17 @@ func (m webFeaturesChromiumHistogramEnumSpannerMapper) GetKeyFromExternal(
return in.WebFeatureID
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) Table() string {
return webFeatureChromiumHistogramEnumValuesTable
func (m webFeaturesChromiumHistogramEnumSpannerMapper) GetKeyFromInternal(
in spannerWebFeatureChromiumHistogramEnum) string {
return in.WebFeatureID
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) Merge(
_ WebFeatureChromiumHistogramEnumValue,
existing spannerWebFeatureChromiumHistogramEnum) spannerWebFeatureChromiumHistogramEnum {
return existing
func (m webFeaturesChromiumHistogramEnumSpannerMapper) Table() string {
return webFeatureChromiumHistogramEnumValuesTable
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) SelectOne(id string) spanner.Statement {
stmt := spanner.NewStatement(fmt.Sprintf(`
SELECT
WebFeatureID, ChromiumHistogramEnumValueID
FROM %s
WHERE WebFeatureID = @webFeatureID
LIMIT 1`, m.Table()))
parameters := map[string]interface{}{
"webFeatureID": id,
}
stmt.Params = parameters

return stmt
func (m webFeaturesChromiumHistogramEnumSpannerMapper) SelectAll() spanner.Statement {
return spanner.NewStatement(fmt.Sprintf(`SELECT * FROM %s`, m.Table()))
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) SelectAllByKeys(id string) spanner.Statement {
Expand All @@ -82,9 +70,37 @@ func (m webFeaturesChromiumHistogramEnumSpannerMapper) SelectAllByKeys(id string
return stmt
}

func (c *Client) UpsertWebFeatureChromiumHistogramEnumValue(
ctx context.Context, in WebFeatureChromiumHistogramEnumValue) error {
return newEntityWriter[webFeaturesChromiumHistogramEnumSpannerMapper](c).upsert(ctx, in)
func (m webFeaturesChromiumHistogramEnumSpannerMapper) MergeAndCheckChanged(
_ WebFeatureChromiumHistogramEnumValue,
existing spannerWebFeatureChromiumHistogramEnum) (spannerWebFeatureChromiumHistogramEnum, bool) {
// This entity only has key columns, so there's nothing to merge or update.
// The synchronizer will handle inserts and deletes based on the key.
return existing, false
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) DeleteMutation(
in spannerWebFeatureChromiumHistogramEnum) *spanner.Mutation {
return spanner.Delete(m.Table(), spanner.Key{in.WebFeatureID})
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) GetChildDeleteKeyMutations(
_ context.Context,
_ *Client,
_ []spannerWebFeatureChromiumHistogramEnum,
) ([]ExtraMutationsGroup, error) {
return nil, nil
}

func (m webFeaturesChromiumHistogramEnumSpannerMapper) PreDeleteHook(
_ context.Context, _ *Client, _ []spannerWebFeatureChromiumHistogramEnum) ([]ExtraMutationsGroup, error) {
return nil, nil
}

func (c *Client) SyncWebFeatureChromiumHistogramEnumValues(
ctx context.Context,
in []WebFeatureChromiumHistogramEnumValue,
) error {
return newEntitySynchronizer[webFeaturesChromiumHistogramEnumSpannerMapper](c).Sync(ctx, in)
}

func (c *Client) getAllWebFeatureChromiumHistogramEnumValuesByFeatureID(
Expand Down
Loading
Loading