Skip to content

Commit

Permalink
RHINENG-8141: make entities unexported, fix and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dugowitch
Dugowitch committed Aug 7, 2024
1 parent 0fe5941 commit f570014
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 47 deletions.
8 changes: 5 additions & 3 deletions evaluator/evaluate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,8 @@ func commitWithObserve(tx *gorm.DB) error {
return nil
}

// First, load advisories and packages (including change evaluation).
// Then, in a single transaction, do the deletions, updates, and insertions.
func evaluateAndStore(system *models.SystemPlatform,
vmaasData *vmaas.UpdatesV3Response, event *mqueue.PlatformEvent) error {
advisoriesByName, err := lazySaveAndLoadAdvisories(system, vmaasData)
Expand Down Expand Up @@ -526,11 +528,11 @@ func analyzeRepos(system *models.SystemPlatform) (thirdParty bool, err error) {

func incrementAdvisoryTypeCounts(advisory models.AdvisoryMetadata, enhCount, bugCount, secCount *int) {
switch advisory.AdvisoryTypeID {
case Enhancement:
case enhancement:
*enhCount++
case Bugfix:
case bugfix:
*bugCount++
case Security:
case security:
*secCount++
}
}
Expand Down
60 changes: 31 additions & 29 deletions evaluator/evaluate_advisories.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import (
"gorm.io/gorm/clause"
)

// Lazy save missing advisories from reported, load stored ones, and evaluate changes between the two.
func lazySaveAndLoadAdvisories(system *models.SystemPlatform, vmaasData *vmaas.UpdatesV3Response) (
ExtendedAdvisoryMap, error) {
extendedAdvisoryMap, error) {
if !enableAdvisoryAnalysis {
utils.LogInfo("advisory analysis disabled, skipping lazy saving and loading")
return nil, nil
Expand All @@ -37,26 +38,27 @@ func lazySaveAndLoadAdvisories(system *models.SystemPlatform, vmaasData *vmaas.U
return merged, nil
}

func pasrseReported(stored SystemAdvisoryMap, reported map[string]int) (ExtendedAdvisoryMap, []string) {
extendedAdvisories := make(ExtendedAdvisoryMap, len(reported)+len(stored))
// Create extendedAdvisoryMap from `stored` and `reported` advisories. It tracks changes between the two.
func pasrseReported(stored SystemAdvisoryMap, reported map[string]int) (extendedAdvisoryMap, []string) {
extendedAdvisories := make(extendedAdvisoryMap, len(reported)+len(stored))
missingNames := make([]string, 0, len(reported))
for reportedName, reportedStatusID := range reported {
if storedAdvisory, found := stored[reportedName]; found {
if reportedStatusID != storedAdvisory.StatusID {
storedAdvisory.StatusID = reportedStatusID
extendedAdvisories[reportedName] = ExtendedAdvisory{
Change: Update,
extendedAdvisories[reportedName] = extendedAdvisory{
change: Update,
SystemAdvisories: storedAdvisory,
}
} else {
extendedAdvisories[reportedName] = ExtendedAdvisory{
Change: Keep,
extendedAdvisories[reportedName] = extendedAdvisory{
change: Keep,
SystemAdvisories: storedAdvisory,
}
}
} else {
extendedAdvisories[reportedName] = ExtendedAdvisory{
Change: Add,
extendedAdvisories[reportedName] = extendedAdvisory{
change: Add,
SystemAdvisories: models.SystemAdvisories{
StatusID: reportedStatusID,
},
Expand All @@ -67,7 +69,7 @@ func pasrseReported(stored SystemAdvisoryMap, reported map[string]int) (Extended
return extendedAdvisories, missingNames
}

func loadMissingNamesIDs(missingNames []string, extendedAdvisories *ExtendedAdvisoryMap) error {
func loadMissingNamesIDs(missingNames []string, extendedAdvisories *extendedAdvisoryMap) error {
advisoryMetadata, err := getAdvisoryMetadataByNames(missingNames)
if err != nil {
return err
Expand All @@ -90,12 +92,12 @@ func loadMissingNamesIDs(missingNames []string, extendedAdvisories *ExtendedAdvi
return nil
}

// Set change for advisories that are in `stored` but not in `reported` to Remove.
func parseStored(stored SystemAdvisoryMap, reported map[string]int, extendedAdvisories *ExtendedAdvisoryMap) {
// ParseStored sets Change for advisories that are in stored but not in reported to Remove.
func parseStored(stored SystemAdvisoryMap, reported map[string]int, extendedAdvisories *extendedAdvisoryMap) {
for storedName, storedAdvisory := range stored {
if _, found := reported[storedName]; !found {
(*extendedAdvisories)[storedName] = ExtendedAdvisory{
Change: Remove,
(*extendedAdvisories)[storedName] = extendedAdvisory{
change: Remove,
SystemAdvisories: storedAdvisory,
}
}
Expand All @@ -104,7 +106,7 @@ func parseStored(stored SystemAdvisoryMap, reported map[string]int, extendedAdvi

// Evaluate changes to all advisories based on `stored` advisories from DB and `reported` advisories from VMaaS.
func evaluateChanges(vmaasData *vmaas.UpdatesV3Response, stored SystemAdvisoryMap) (
ExtendedAdvisoryMap, error) {
extendedAdvisoryMap, error) {
reported := getReportedAdvisories(vmaasData)
extendedAdvisories, missingNames := pasrseReported(stored, reported)

Expand Down Expand Up @@ -206,7 +208,7 @@ func getMissingAdvisories(advisoryNames []string) ([]string, error) {
return missingNames, nil
}

func storeAdvisoryData(tx *gorm.DB, system *models.SystemPlatform, advisoriesByName ExtendedAdvisoryMap) (
func storeAdvisoryData(tx *gorm.DB, system *models.SystemPlatform, advisoriesByName extendedAdvisoryMap) (
SystemAdvisoryMap, error) {
if !enableAdvisoryAnalysis {
utils.LogInfo("advisory analysis disabled, skipping storing")
Expand All @@ -229,7 +231,7 @@ func storeAdvisoryData(tx *gorm.DB, system *models.SystemPlatform, advisoriesByN

func calcAdvisoryChanges(system *models.SystemPlatform, deleteIDs []int64,
systemAdvisories SystemAdvisoryMap) []models.AdvisoryAccountData {
// If system is stale, we won't change any rows in advisory_account_data
// If system is stale, we won't change any rows in advisory_account_data
if system.Stale {
return []models.AdvisoryAccountData{}
}
Expand Down Expand Up @@ -295,13 +297,13 @@ func upsertSystemAdvisories(tx *gorm.DB, advisoryObjs models.SystemAdvisoriesSli
return database.BulkInsert(tx, advisoryObjs)
}

func processAdvisories(system *models.SystemPlatform, advisoriesByName ExtendedAdvisoryMap) ([]int64,
func processAdvisories(system *models.SystemPlatform, advisoriesByName extendedAdvisoryMap) ([]int64,
models.SystemAdvisoriesSlice, SystemAdvisoryMap) {
deleteIDs := make([]int64, 0, len(advisoriesByName))
advisoryObjs := make(models.SystemAdvisoriesSlice, 0, len(advisoriesByName))
updatedAdvisories := make(SystemAdvisoryMap, len(advisoriesByName))
for name, advisory := range advisoriesByName {
switch advisory.Change {
switch advisory.change {
case Remove:
deleteIDs = append(deleteIDs, advisory.AdvisoryID)
case Update:
Expand All @@ -325,7 +327,7 @@ func processAdvisories(system *models.SystemPlatform, advisoriesByName ExtendedA
}

func updateSystemAdvisories(tx *gorm.DB, system *models.SystemPlatform,
advisoriesByName ExtendedAdvisoryMap) ([]int64, SystemAdvisoryMap, error) {
advisoriesByName extendedAdvisoryMap) ([]int64, SystemAdvisoryMap, error) {
deleteIDs, advisoryObjs, updatedAdvisories := processAdvisories(system, advisoriesByName)

// this will remove many many old items from our "system_advisories" table
Expand Down Expand Up @@ -356,8 +358,8 @@ func loadSystemAdvisories(accountID int, systemID int64) (SystemAdvisoryMap, err
return systemAdvisories, nil
}

func updateAdvisoryAccountData(tx *gorm.DB, system *models.SystemPlatform,
deleteIDs []int64, systemAdvisories SystemAdvisoryMap) error {
func updateAdvisoryAccountData(tx *gorm.DB, system *models.SystemPlatform, deleteIDs []int64,
systemAdvisories SystemAdvisoryMap) error {
changes := calcAdvisoryChanges(system, deleteIDs, systemAdvisories)

if len(changes) == 0 {
Expand All @@ -373,16 +375,16 @@ func updateAdvisoryAccountData(tx *gorm.DB, system *models.SystemPlatform,
return database.BulkInsert(txOnConflict, changes)
}

type ExtendedAdvisory struct {
Change ChangeType
type extendedAdvisory struct {
change ChangeType
models.SystemAdvisories
}

type ExtendedAdvisoryMap map[string]ExtendedAdvisory
type extendedAdvisoryMap map[string]extendedAdvisory

const (
UnknownFixme int = iota
Enhancement
Bugfix
Security
undefinedChangeType int = iota
enhancement
bugfix
security
)
68 changes: 53 additions & 15 deletions evaluator/evaluate_advisories_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ func TestEvaluateChanges(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, 4, len(extendedAdvisories))

assert.Equal(t, Keep, extendedAdvisories["ER-1"].Change)
assert.Equal(t, Remove, extendedAdvisories["ER-2"].Change)
assert.Equal(t, Update, extendedAdvisories["ER-3"].Change)
assert.Equal(t, Add, extendedAdvisories["ER-4"].Change)
assert.Equal(t, Keep, extendedAdvisories["ER-1"].change)
assert.Equal(t, Remove, extendedAdvisories["ER-2"].change)
assert.Equal(t, Update, extendedAdvisories["ER-3"].change)
assert.Equal(t, Add, extendedAdvisories["ER-4"].change)
}

func TestLoadMissingNamesIDs(t *testing.T) {
Expand All @@ -110,7 +110,7 @@ func TestLoadMissingNamesIDs(t *testing.T) {

vmaasData := mockVMaaSResponse()
missingNames := []string{"ER1", "ER2", "ER3", "ER4"}
extendedAdvisories := ExtendedAdvisoryMap{"ER1": {}, "ER2": {}, "ER3": {}, "ER4": {}}
extendedAdvisories := extendedAdvisoryMap{"ER1": {}, "ER2": {}, "ER3": {}, "ER4": {}}

// test error if not lazy saved
err := loadMissingNamesIDs(missingNames, &extendedAdvisories)
Expand All @@ -128,21 +128,59 @@ func TestLoadMissingNamesIDs(t *testing.T) {
assert.NotEqual(t, int64(0), extendedAdvisories["ER4"].AdvisoryID)
}

func TestParseReported(t *testing.T) {
stored := SystemAdvisoryMap{
"ER-42": models.SystemAdvisories{StatusID: INSTALLABLE},
"ER-43": models.SystemAdvisories{StatusID: INSTALLABLE},
}
reported := map[string]int{
"ER-42": INSTALLABLE,
"ER-43": APPLICABLE,
"ER-44": INSTALLABLE,
}
extendedAdvisories, missingNames := pasrseReported(stored, reported)
assert.Equal(t, 3, len(extendedAdvisories))
assert.Equal(t, Keep, extendedAdvisories["ER-42"].change)
assert.Equal(t, Update, extendedAdvisories["ER-43"].change)
assert.Equal(t, Add, extendedAdvisories["ER-44"].change)
assert.Equal(t, []string{"ER-44"}, missingNames)
}

func TestParseStored(t *testing.T) {
stored := SystemAdvisoryMap{
"ER-42": models.SystemAdvisories{},
"ER-43": models.SystemAdvisories{},
"ER-43": models.SystemAdvisories{StatusID: INSTALLABLE},
}
reported := map[string]int{
"ER-43": 43,
"ER-43": INSTALLABLE,
}
extendedAdvisories := ExtendedAdvisoryMap{
"ER-43": ExtendedAdvisory{Change: Keep, SystemAdvisories: stored["ER-43"]},
extendedAdvisories := extendedAdvisoryMap{
"ER-43": extendedAdvisory{change: Keep, SystemAdvisories: stored["ER-43"]},
}
parseStored(stored, reported, &extendedAdvisories)
assert.Equal(t, 2, len(extendedAdvisories))
assert.Equal(t, Remove, extendedAdvisories["ER-42"].Change)
assert.Equal(t, Keep, extendedAdvisories["ER-43"].Change)
assert.Equal(t, Remove, extendedAdvisories["ER-42"].change)
assert.Equal(t, Keep, extendedAdvisories["ER-43"].change)
}

func TestIncrementAdvisoryTypeCounts(t *testing.T) {
var (
enhCount int
bugCount int
secCount int
)
advisories := []models.AdvisoryMetadata{
{AdvisoryTypeID: enhancement},
{AdvisoryTypeID: bugfix},
{AdvisoryTypeID: security},
}

for _, advisory := range advisories {
incrementAdvisoryTypeCounts(advisory, &enhCount, &bugCount, &secCount)
}
assert.Equal(t, 1, enhCount)
assert.Equal(t, 1, bugCount)
assert.Equal(t, 1, secCount)
}

func TestUpdateAdvisoryAccountData(t *testing.T) {
Expand Down Expand Up @@ -203,14 +241,14 @@ func TestProcessAdvisories(t *testing.T) {

systemID := int64(2)
system := models.SystemPlatform{RhAccountID: 1, ID: systemID}
extendedAdvisories := ExtendedAdvisoryMap{
"ER-2": ExtendedAdvisory{Change: Keep, SystemAdvisories: models.SystemAdvisories{
extendedAdvisories := extendedAdvisoryMap{
"ER-2": extendedAdvisory{change: Keep, SystemAdvisories: models.SystemAdvisories{
AdvisoryID: int64(2),
}},
"ER-3": ExtendedAdvisory{Change: Add, SystemAdvisories: models.SystemAdvisories{
"ER-3": extendedAdvisory{change: Add, SystemAdvisories: models.SystemAdvisories{
AdvisoryID: int64(3),
}},
"ER-4": ExtendedAdvisory{Change: Update, SystemAdvisories: models.SystemAdvisories{
"ER-4": extendedAdvisory{change: Update, SystemAdvisories: models.SystemAdvisories{
AdvisoryID: int64(4),
}},
}
Expand Down

0 comments on commit f570014

Please sign in to comment.