Skip to content

Commit 5bd7c64

Browse files
committed
session: fix empty config for features in sql mig
In the bbolt store, an empty config for a feature is represented as an empty array, while in for the SQL store, the same config is represented as nil. Therefore, in the scenario where a specific feature has an empty config, we override the SQL FeatureConfig for that feature to also be set to an empty array. This is needed to ensure that the deep equals check in the migration validation does not fail in this scenario.
1 parent e3a8589 commit 5bd7c64

File tree

2 files changed

+86
-3
lines changed

2 files changed

+86
-3
lines changed

session/sql_migration.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ func migrateSessionsToSQLAndValidate(ctx context.Context,
198198
overrideSessionTimeZone(migratedSession)
199199
overrideMacaroonRecipe(kvSession, migratedSession)
200200
overrideRemovedAccount(kvSession, migratedSession)
201+
overrideFeatureConfig(kvSession, migratedSession)
201202

202203
if !reflect.DeepEqual(kvSession, migratedSession) {
203204
diff := difflib.UnifiedDiff{
@@ -530,3 +531,30 @@ func overrideRemovedAccount(kvSession *Session, migratedSession *Session) {
530531
},
531532
)
532533
}
534+
535+
// overrideFeatureConfig overrides a specific feature's config for the SQL
536+
// session in a certain scenario:
537+
//
538+
// In the bbolt store, an empty config for a feature is represented as an empty
539+
// array, while in for the SQL store, the same config is represented as nil.
540+
// Therefore, in the scenario where a specific feature has an empty config, we
541+
// override the SQL FeatureConfig for that feature to also be set to an empty
542+
// array. This is needed to ensure that the deep equals check in the migration
543+
// validation does not fail in this scenario.
544+
func overrideFeatureConfig(kvSession *Session, mSession *Session) {
545+
// If the FeatureConfig hasn't been set, we can return early.
546+
if mSession.FeatureConfig == nil {
547+
return
548+
}
549+
migratedConf := *mSession.FeatureConfig
550+
551+
if kvSession.FeatureConfig != nil && mSession.FeatureConfig != nil {
552+
for featureName, config := range *kvSession.FeatureConfig {
553+
// If the config is empty for the feature, we override
554+
// the SQL version.
555+
if len(config) == 0 {
556+
migratedConf[featureName] = make([]byte, 0)
557+
}
558+
}
559+
}
560+
}

session/sql_migration_test.go

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ func TestSessionsStoreMigration(t *testing.T) {
8181
// session doesn't.
8282
overrideRemovedAccount(kvSession, sqlSession)
8383

84+
// Finally, we also need to override specific feature's
85+
// config if they are empty.
86+
overrideFeatureConfig(kvSession, sqlSession)
87+
8488
assertEqualSessions(t, kvSession, sqlSession)
8589
}
8690

@@ -200,6 +204,48 @@ func TestSessionsStoreMigration(t *testing.T) {
200204
return getBoltStoreSessions(t, store)
201205
},
202206
},
207+
{
208+
name: "one session with a feature config with empty",
209+
populateDB: func(t *testing.T, store *BoltStore,
210+
_ accounts.Store) []*Session {
211+
212+
// Set a feature config, which has an empty
213+
// entry for a specific feature.
214+
featureConfig := map[string][]byte{
215+
"AutoFees": {},
216+
}
217+
218+
_, err := store.NewSession(
219+
ctx, "test", TypeMacaroonAdmin,
220+
time.Unix(1000, 0), "",
221+
WithFeatureConfig(featureConfig),
222+
)
223+
require.NoError(t, err)
224+
225+
return getBoltStoreSessions(t, store)
226+
},
227+
},
228+
{
229+
name: "one session with a feature config with nil",
230+
populateDB: func(t *testing.T, store *BoltStore,
231+
_ accounts.Store) []*Session {
232+
233+
// Set a feature config, which has a nil entry
234+
// for a specific feature.
235+
featureConfig := map[string][]byte{
236+
"AutoFees": nil,
237+
}
238+
239+
_, err := store.NewSession(
240+
ctx, "test", TypeMacaroonAdmin,
241+
time.Unix(1000, 0), "",
242+
WithFeatureConfig(featureConfig),
243+
)
244+
require.NoError(t, err)
245+
246+
return getBoltStoreSessions(t, store)
247+
},
248+
},
203249
{
204250
name: "one session with dev server",
205251
populateDB: func(t *testing.T, store *BoltStore,
@@ -801,9 +847,18 @@ func randomPrivacyFlags() PrivacyFlags {
801847
func randomFeatureConfig() FeaturesConfig {
802848
featureConfig := make(FeaturesConfig)
803849
for i := range rand.Intn(10) {
804-
featureName := fmt.Sprintf("feature%d", i)
805-
featureValue := []byte{byte(rand.Int31())}
806-
featureConfig[featureName] = featureValue
850+
var (
851+
featureName = fmt.Sprintf("feature%d", i)
852+
configValue []byte
853+
)
854+
855+
// For the vast majority of the features, we set a value for the
856+
// feature's config. But in 1/5 of the cases, we set an empty
857+
// config.
858+
if rand.Intn(5) != 0 {
859+
configValue = []byte{byte(rand.Int31())}
860+
}
861+
featureConfig[featureName] = configValue
807862
}
808863

809864
return featureConfig

0 commit comments

Comments
 (0)