Merge pull request #243 from eurofurence/issue-239-multi-packages

improve package encoding in DB to support large number of packages and larger counts

internal/entity/attendee.go

@@ -25,11 +25,11 @@ type Attendee struct {
 	Gender               string `gorm:"type:varchar(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;NOT NULL"`
 	Pronouns             string `gorm:"type:varchar(40) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"`
 	TshirtSize           string `gorm:"type:varchar(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"`
-	SpokenLanguages      string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
-	RegistrationLanguage string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
-	Flags                string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
-	Packages             string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
-	Options              string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
+	SpokenLanguages      string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"`  // comma-separated choice field with leading and trailing comma
+	RegistrationLanguage string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"`  // comma-separated choice field with leading and trailing comma
+	Flags                string `gorm:"type:varchar(4096) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
+	Packages             string `gorm:"type:varchar(4096) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma, each entry can contain :count postfix
+	Options              string `gorm:"type:varchar(4096) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci"` // comma-separated choice field with leading and trailing comma
 	UserComments         string `gorm:"type:text CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci" testdiff:"ignore"`
 	Identity             string `gorm:"type:varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci;uniqueIndex:att_attendees_identity_uidx"`
 	CacheTotalDues       int64  `testdiff:"ignore"`                                                                          // cache for search functionality only: valid dues balance

internal/repository/config/config.go

@@ -214,6 +214,7 @@ func DefaultFlags() string {
 }
 
 func DefaultPackages() string {
+	// this is ok because all our parsing implementations can deal with a simple comma separated list
 	return defaultChoiceStr(Configuration().Choices.Packages)
 }
 

internal/repository/database/historizeddb/implementation_test.go

@@ -32,7 +32,7 @@ func tstBuildValidAttendee() *entity.Attendee {
 		SpokenLanguages:      ",de,en,",
 		RegistrationLanguage: ",en-US,",
 		Flags:                ",anon,ev,",
-		Packages:             ",room-none,attendance,stage,sponsor2,",
+		Packages:             ",room-none:1,attendance:1,stage:1,sponsor2:1,",
 		Options:              ",music,suit,",
 		TshirtSize:           "XXL",
 		UserComments:         "this is a comment",

internal/repository/database/inmemorydb/match.go

@@ -1,12 +1,14 @@
 package inmemorydb
 
 import (
+	aulogging "github.com/StephanHCB/go-autumn-logging"
 	"github.com/eurofurence/reg-attendee-service/internal/api/v1/attendee"
 	"github.com/eurofurence/reg-attendee-service/internal/api/v1/status"
 	"github.com/eurofurence/reg-attendee-service/internal/entity"
 	"github.com/eurofurence/reg-attendee-service/internal/repository/config"
 	"github.com/eurofurence/reg-attendee-service/internal/web/util/validation"
 	"github.com/ryanuber/go-glob"
+	"strconv"
 	"strings"
 )
 
@@ -65,29 +67,62 @@ func matchesExactOrEmpty(cond string, value string) bool {
 	return cond == "" || cond == value
 }
 
-func choiceMatch(cond map[string]int8, rawValues ...string) bool {
-	combined := ""
-	for _, rawValue := range rawValues {
-		value := strings.TrimPrefix(rawValue, ",")
-		value = strings.TrimSuffix(value, ",")
-		combined = combined + value + ","
-	}
-	combined = strings.TrimSuffix(combined, ",")
-
-	chosen := strings.Split(combined, ",")
+func choiceMatch(cond map[string]int8, selectedValues ...string) bool {
+	chosen := choiceCountMap(selectedValues...)
 
 	for k, v := range cond {
-		contained := validation.SliceContains(chosen, k)
-		if v == 1 && !contained {
+		count, _ := chosen[k]
+		if v == 1 && count == 0 {
 			return false
 		}
-		if v == 0 && contained {
+		if v == 0 && count > 0 {
 			return false
 		}
 	}
 	return true
 }
 
+// choiceCountMap allows passing in multiple dbRepresentations that are
+// combined into a single count map.
+//
+// Used to combine flags and admin flags into a single map.
+//
+// Each parameter is a comma separated list of choice names, possibly followed
+// by :count, where count is a positive integer. If the :count postfix is missing,
+// it is treated as a count of 1.
+//
+// The :count postfix is currently only in use for packages.
+func choiceCountMap(dbRepresentations ...string) map[string]int {
+	result := make(map[string]int)
+
+	for _, dbRepr := range dbRepresentations {
+		value := strings.TrimPrefix(dbRepr, ",")
+		value = strings.TrimSuffix(value, ",")
+
+		chosen := strings.Split(value, ",")
+
+		for _, entry := range chosen {
+			if entry != "" {
+				nameAndPossiblyCount := strings.Split(entry, ":")
+				name := nameAndPossiblyCount[0]
+				count := 1
+				if len(nameAndPossiblyCount) > 1 {
+					var err error
+					count, err = strconv.Atoi(nameAndPossiblyCount[1])
+					if err != nil {
+						aulogging.Logger.NoCtx().Warn().Printf("encountered invalid choice entry '%s' in database - ignoring", entry)
+						continue
+					}
+				}
+				currentCount, _ := result[name]
+				result[name] = currentCount + count
+			}
+		}
+	}
+
+	return result
+}
+
 func matchesStatus(wanted []status.Status, value status.Status) bool {
 	if len(wanted) == 0 {
 		// default: all except deleted

internal/repository/database/inmemorydb/match_test.go

@@ -15,3 +15,15 @@ func TestMatchesIsoDateRange(t *testing.T) {
 	require.False(t, matchesIsoDateRange("1976-10-22", "1977-01-01", "1972-12-24"))
 	require.False(t, matchesIsoDateRange("1976-10-22", "1977-01-01", "1979-12-24"))
 }
+
+func TestChoiceCountMap(t *testing.T) {
+	actual := choiceCountMap(",a,b:5,", ",c:17,d,e:1,")
+	expected := map[string]int{
+		"a": 1,
+		"b": 5,
+		"c": 17,
+		"d": 1,
+		"e": 1,
+	}
+	require.EqualValues(t, expected, actual)
+}

internal/repository/database/mysqldb/searchquery.go

@@ -239,7 +239,7 @@ func (r *MysqlRepository) addSingleCondition(cond *attendee.AttendeeSearchSingle
 	query.WriteString(choiceMatch("a.registration_language", cond.RegistrationLanguage, params, paramBaseName, &paramNo))
 	query.WriteString(choiceMatch("CONCAT(a.flags,IFNULL(ad.flags, ''))", cond.Flags, params, paramBaseName, &paramNo))
 	query.WriteString(choiceMatch("a.options", cond.Options, params, paramBaseName, &paramNo))
-	query.WriteString(choiceMatch("a.packages", cond.Packages, params, paramBaseName, &paramNo))
+	query.WriteString(packageMatch("a.packages", cond.Packages, params, paramBaseName, &paramNo))
 	if cond.UserComments != "" {
 		query.WriteString(substringMatch("a.user_comments", cond.UserComments, params, paramBaseName, &paramNo))
 	}
@@ -365,6 +365,25 @@ func choiceMatch(field string, condition map[string]int8, params map[string]inte
 	return query.String()
 }
 
+func packageMatch(field string, condition map[string]int8, params map[string]interface{}, paramBaseName string, idx *int) string {
+	query := strings.Builder{}
+	keys := sortedKeySet(condition)
+
+	for _, k := range keys {
+		pName := fmt.Sprintf("%s_%d_nc", paramBaseName, *idx)
+		params[pName] = "%," + k + ",%" // version without a count postfix
+		pName2 := fmt.Sprintf("%s_%d", paramBaseName, *idx)
+		params[pName2] = "%," + k + ":%" // version with a count postfix
+		if condition[k] == 1 {
+			query.WriteString(fmt.Sprintf("    AND ( ( %s LIKE @%s ) OR ( %s LIKE @%s ) )\n", field, pName, field, pName2))
+		} else if condition[k] == 0 {
+			query.WriteString(fmt.Sprintf("    AND ( %s NOT LIKE @%s ) AND ( %s NOT LIKE @%s )\n", field, pName, field, pName2))
+		}
+		*idx++
+	}
+	return query.String()
+}
+
 func sortedKeySet(condition map[string]int8) []string {
 	keys := make([]string, len(condition))
 	i := 0

internal/repository/database/mysqldb/searchquery_test.go

@@ -148,8 +148,10 @@ func TestTwoFullSearchQueries(t *testing.T) {
 		"param_1_12":                        "%,flagzero,%",
 		"param_1_13":                        "%,optone,%",
 		"param_1_14":                        "%,optzero,%",
-		"param_1_15":                        "%,pkgone,%",
-		"param_1_16":                        "%,pkgzero,%",
+		"param_1_15_nc":                     "%,pkgone,%",
+		"param_1_15":                        "%,pkgone:%",
+		"param_1_16_nc":                     "%,pkgzero,%",
+		"param_1_16":                        "%,pkgzero:%",
 		"param_1_17":                        "%user%comments%",
 		"param_1_18_1":                      "2020-12-23",
 		"param_1_18_2":                      "sponsor-items",
@@ -168,8 +170,10 @@ func TestTwoFullSearchQueries(t *testing.T) {
 		"param_2_12":                        "%,fzero,%",
 		"param_2_13":                        "%,oone,%",
 		"param_2_14":                        "%,ozero,%",
-		"param_2_15":                        "%,pone,%",
-		"param_2_16":                        "%,pzero,%",
+		"param_2_15_nc":                     "%,pone,%",
+		"param_2_15":                        "%,pone:%",
+		"param_2_16_nc":                     "%,pzero,%",
+		"param_2_16":                        "%,pzero:%",
 		"param_2_17":                        "%more user comments%",
 		"param_2_18_1":                      "2020-12-23",
 		"param_2_18_2":                      "sponsor-items",
@@ -199,8 +203,8 @@ WHERE (
     AND ( CONCAT(a.flags,IFNULL(ad.flags, '')) NOT LIKE @param_1_12 )
     AND ( a.options LIKE @param_1_13 )
     AND ( a.options NOT LIKE @param_1_14 )
-    AND ( a.packages LIKE @param_1_15 )
-    AND ( a.packages NOT LIKE @param_1_16 )
+    AND ( ( a.packages LIKE @param_1_15_nc ) OR ( a.packages LIKE @param_1_15 ) )
+    AND ( a.packages NOT LIKE @param_1_16_nc ) AND ( a.packages NOT LIKE @param_1_16 )
     AND ( LOWER(a.user_comments) LIKE LOWER( @param_1_17 ) )
     AND ( IFNULL(st.status, 'new') <> 'deleted' )
     AND ( STRCMP( IFNULL(a.cache_due_date,'9999-99-99'), @param_1_18_1 ) >= 0 )
@@ -225,8 +229,8 @@ WHERE (
     AND ( CONCAT(a.flags,IFNULL(ad.flags, '')) NOT LIKE @param_2_12 )
     AND ( a.options LIKE @param_2_13 )
     AND ( a.options NOT LIKE @param_2_14 )
-    AND ( a.packages LIKE @param_2_15 )
-    AND ( a.packages NOT LIKE @param_2_16 )
+    AND ( ( a.packages LIKE @param_2_15_nc ) OR ( a.packages LIKE @param_2_15 ) )
+    AND ( a.packages NOT LIKE @param_2_16_nc ) AND ( a.packages NOT LIKE @param_2_16 )
     AND ( LOWER(a.user_comments) LIKE LOWER( @param_2_17 ) )
     AND ( IFNULL(st.status, 'new') <> 'deleted' )
     AND ( STRCMP( IFNULL(a.cache_due_date,'9999-99-99'), @param_2_18_1 ) < 0 )

internal/service/attendeesrv/attendeesrv.go

@@ -12,6 +12,7 @@ import (
 	"github.com/eurofurence/reg-attendee-service/internal/repository/database"
 	"github.com/eurofurence/reg-attendee-service/internal/web/util/ctxvalues"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 )
@@ -316,23 +317,64 @@ func checkNoConstraintViolation(key string, choiceConfig config.ChoiceConfig, ne
 // choiceStrToMap converts a choice representation in the entity to a map of counts
 //
 // Can be used for packages, flags, options.
+//
+// choiceStr is a comma separated list of choice names, each possibly followed
+// by :count, where count is a positive integer. If the :count postfix is missing,
+// it is treated as a count of 1.
+//
+// The :count postfix is currently only in use for packages.
+//
+// It is ok for the same choice name to occur multiple times in the list. This is in order to
+// remain backwards compatible when processing requests not using the new packages_list field,
+// and in order to allow adding packages directly in the database by just appending "packagename,",
+// which is sometimes incredibly useful.
 func choiceStrToMap(choiceStr string, configuration map[string]config.ChoiceConfig) map[string]int {
-	result := make(map[string]int)
+	result := choiceStrToMapWithoutChecks(choiceStr)
 	// ensure all available keys present
 	for k, _ := range configuration {
-		result[k] = 0
+		if _, ok := result[k]; !ok {
+			result[k] = 0
+		}
 	}
+	// warn for counts exceeding MaxCount
+	for name, count := range result {
+		conf, ok := configuration[name]
+		if !ok {
+			aulogging.Logger.NoCtx().Warn().Printf("encountered non-configured choice key %s - maybe configuration changed after initial reg? This needs fixing! - continuing", name)
+		}
+		if conf.MaxCount == 0 {
+			conf.MaxCount = 1
+		}
+		if count > conf.MaxCount {
+			aulogging.Logger.NoCtx().Warn().Printf("encountered choice key %s with excessive count %d - maybe configuration changed after initial reg? This needs fixing! - continuing", name, count)
+		}
+	}
+	return result
+}
+
+// choiceStrToMapWithoutChecks converts a choice representation in the entity to a map of counts.
+//
+// Low level version without validation against a choice configuration.
+func choiceStrToMapWithoutChecks(choiceStr string) map[string]int {
+	result := make(map[string]int)
 	if choiceStr != "" {
 		choices := strings.Split(choiceStr, ",")
-		for _, pickedKey := range choices {
-			if pickedKey != "" {
-				currentValue, present := result[pickedKey]
-				if present {
-					result[pickedKey] = currentValue + 1
-				} else {
-					aulogging.Logger.NoCtx().Warn().Printf("encountered non-configured choice key %s - maybe configuration changed after initial reg? This needs fixing! - continuing", pickedKey)
-					result[pickedKey] = 1
+		for _, entry := range choices {
+			if entry != "" {
+				nameAndPossiblyCount := strings.Split(entry, ":")
+				name := nameAndPossiblyCount[0]
+				count := 1
+				if len(nameAndPossiblyCount) > 1 {
+					var err error
+					count, err = strconv.Atoi(nameAndPossiblyCount[1])
+					if err != nil {
+						aulogging.Logger.NoCtx().Warn().Printf("encountered invalid choice entry '%s' in database - ignoring (please fix!)", entry)
+						continue
+					}
 				}
+
+				currentValue, _ := result[name]
+				result[name] = currentValue + count
 			}
 		}
 	}
@@ -349,15 +391,16 @@ func choiceListToMap(choiceList []attendee.PackageState, configuration map[strin
 		result[k] = 0
 	}
 	for _, entry := range choiceList {
+		if entry.Count == 0 {
+			entry.Count = 1
+		}
+
 		currentValue, present := result[entry.Name]
 		if present {
-			if entry.Count == 0 {
-				entry.Count = 1
-			}
 			result[entry.Name] = currentValue + entry.Count
 		} else {
 			aulogging.Logger.NoCtx().Warn().Printf("encountered non-configured choice key '%s' - maybe configuration changed after initial reg? This needs fixing! - continuing", entry.Name)
-			result[entry.Name] = 1
+			result[entry.Name] = entry.Count
 		}
 	}
 	return result

internal/service/attendeesrv/attendeesrv_test.go

@@ -0,0 +1,39 @@
+package attendeesrv
+
+import (
+	"github.com/eurofurence/reg-attendee-service/internal/repository/config"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+var tstChoiceConfig = map[string]config.ChoiceConfig{
+	"a": {
+		MaxCount: 0, // interpreted as 1
+	},
+	"b": {
+		MaxCount: 1,
+	},
+	"c": {
+		MaxCount: 4,
+	},
+}
+
+func TestChoiceStrToMap_Flags(t *testing.T) {
+	actual := choiceStrToMap(",a,b,", tstChoiceConfig)
+	expected := map[string]int{
+		"a": 1,
+		"b": 1,
+		"c": 0,
+	}
+	require.EqualValues(t, expected, actual)
+}
+
+func TestChoiceStrToMap_Packages(t *testing.T) {
+	actual := choiceStrToMap(",a:1,c,b,c:2,", tstChoiceConfig)
+	expected := map[string]int{
+		"a": 1,
+		"b": 1,
+		"c": 3,
+	}
+	require.EqualValues(t, expected, actual)
+}

internal/service/attendeesrv/generate.go

@@ -56,7 +56,7 @@ func fakeRegistration() *entity.Attendee {
 		SpokenLanguages:      "," + randomSelection(config.AllowedSpokenLanguages(), 1, 5) + ",",
 		RegistrationLanguage: oneOf(config.AllowedRegistrationLanguages()),
 		Flags:                ",terms-accepted," + randomSelection([]string{"hc", "anon", "digi-book"}, 1, 3) + ",",
-		Packages:             ",room-none,attendance,stage," + oneOf([]string{"sponsor", "sponsor2", "tshirt"}) + ",",
+		Packages:             ",room-none:1,attendance:1,stage:1," + oneOf([]string{"sponsor:1", "sponsor2:1", "tshirt:1"}) + ",",
 		Options:              "," + randomSelection(config.AllowedOptions(), 1, 4) + ",",
 		UserComments:         "generated by load test",
 		Identity:             randomString(10, 12, 0) + "_gen",
@@ -168,7 +168,7 @@ func mapAttendeeToDto(a *entity.Attendee, dto *attendee.AttendeeDto) {
 	dto.SpokenLanguages = removeWrappingCommas(a.SpokenLanguages)
 	dto.RegistrationLanguage = removeWrappingCommas(a.RegistrationLanguage)
 	dto.Flags = removeWrappingCommas(a.Flags)
-	dto.PackagesList = sortedPackageListFromCommaSeparated(removeWrappingCommas(a.Packages))
+	dto.PackagesList = sortedPackageListFromCommaSeparatedWithCounts(removeWrappingCommas(a.Packages))
 	dto.Options = removeWrappingCommas(a.Options)
 	dto.UserComments = a.UserComments
 }