From 38c4828f2fb69e89d5355805122525e536ca94fe Mon Sep 17 00:00:00 2001 From: Sam Lucidi Date: Thu, 28 Sep 2023 12:51:46 -0400 Subject: [PATCH 1/2] Exclude virtual tags from writing/updating * Add `virtual` field to TagRef * Ignore tag refs with `virtual` field set true when adding, updating, or replacing tags. * Fix bug preventing archetype tags being inherited properly * go fmt Signed-off-by: Sam Lucidi --- Makefile | 2 + api/application.go | 90 +++++++++++++++++++++++++++-------- api/base.go | 10 ++-- assessment/application.go | 14 +++--- assessment/assessment_test.go | 2 +- assessment/membership.go | 12 ++--- assessment/pkg.go | 16 +++---- assessment/questionnaire.go | 6 +-- assessment/section.go | 1 - assessment/tag.go | 4 +- metrics/manager.go | 2 +- metrics/pkg.go | 2 +- 12 files changed, 107 insertions(+), 54 deletions(-) diff --git a/Makefile b/Makefile index 8333d0856..b65f6ba8d 100644 --- a/Makefile +++ b/Makefile @@ -4,12 +4,14 @@ HUB_BASE_URL ?= http://localhost:8080 PKG = ./addon/... \ ./api/... \ + ./assessment/... \ ./auth/... \ ./cmd/... \ ./database/... \ ./encryption/... \ ./importer/... \ ./k8s/... \ + ./metrics/... \ ./migration/... \ ./model/... \ ./settings/... \ diff --git a/api/application.go b/api/application.go index 3b27f38a0..6fba7581f 100644 --- a/api/application.go +++ b/api/application.go @@ -32,6 +32,13 @@ const ( Source = "source" ) +// +// Tag Sources +const ( + SourceAssessment = "assessment" + SourceArchetype = "archetype" +) + // // ApplicationHandler handles application resource routes. type ApplicationHandler struct { @@ -140,8 +147,8 @@ func (h ApplicationHandler) Get(ctx *gin.Context) { r := Application{} r.With(m, tags) r.WithArchetypes(archetypes) - r.WithSourcedTags(archetypeTags, "archetype") - r.WithSourcedTags(resolver.AssessmentTags(), "assessment") + r.WithVirtualTags(archetypeTags, "archetype") + r.WithVirtualTags(resolver.AssessmentTags(), "assessment") r.Assessed, err = resolver.Assessed() if err != nil { _ = ctx.Error(err) @@ -203,8 +210,8 @@ func (h ApplicationHandler) List(ctx *gin.Context) { r := Application{} r.With(&list[i], tags) r.WithArchetypes(archetypes) - r.WithSourcedTags(archetypeTags, "archetype") - r.WithSourcedTags(resolver.AssessmentTags(), "assessment") + r.WithVirtualTags(archetypeTags, "archetype") + r.WithVirtualTags(resolver.AssessmentTags(), "assessment") r.Assessed, err = resolver.Assessed() if err != nil { _ = ctx.Error(err) @@ -243,7 +250,9 @@ func (h ApplicationHandler) Create(ctx *gin.Context) { tags := []model.ApplicationTag{} if len(r.Tags) > 0 { for _, t := range r.Tags { - tags = append(tags, model.ApplicationTag{TagID: t.ID, ApplicationID: m.ID, Source: t.Source}) + if !t.Virtual { + tags = append(tags, model.ApplicationTag{TagID: t.ID, ApplicationID: m.ID, Source: t.Source}) + } } result = h.DB(ctx).Create(&tags) if result.Error != nil { @@ -277,8 +286,8 @@ func (h ApplicationHandler) Create(ctx *gin.Context) { r.With(m, tags) r.WithArchetypes(archetypes) - r.WithSourcedTags(archetypeTags, "archetype") - r.WithSourcedTags(resolver.AssessmentTags(), "assessment") + r.WithVirtualTags(archetypeTags, "archetype") + r.WithVirtualTags(resolver.AssessmentTags(), "assessment") r.Assessed, err = resolver.Assessed() if err != nil { _ = ctx.Error(err) @@ -399,7 +408,9 @@ func (h ApplicationHandler) Update(ctx *gin.Context) { if len(r.Tags) > 0 { tags := []model.ApplicationTag{} for _, t := range r.Tags { - tags = append(tags, model.ApplicationTag{TagID: t.ID, ApplicationID: m.ID, Source: t.Source}) + if !t.Virtual { + tags = append(tags, model.ApplicationTag{TagID: t.ID, ApplicationID: m.ID, Source: t.Source}) + } } result = h.DB(ctx).Create(&tags) if result.Error != nil { @@ -492,12 +503,12 @@ func (h ApplicationHandler) BucketDelete(ctx *gin.Context) { // @tags applications // @produce json // @success 200 {object} []api.Ref -// @router /applications/{id}/tags/id [get] +// @router /applications/{id}/tags [get] // @param id path string true "Application ID" func (h ApplicationHandler) TagList(ctx *gin.Context) { id := h.pk(ctx) app := &model.Application{} - result := h.DB(ctx).First(app, id) + result := h.DB(ctx).Preload("Tags").First(app, id) if result.Error != nil { _ = ctx.Error(result.Error) return @@ -518,9 +529,41 @@ func (h ApplicationHandler) TagList(ctx *gin.Context) { resources := []TagRef{} for i := range list { r := TagRef{} - r.With(list[i].Tag.ID, list[i].Tag.Name, list[i].Source) + r.With(list[i].Tag.ID, list[i].Tag.Name, list[i].Source, false) resources = append(resources, r) } + + includeAssessment := !found || source == SourceAssessment + includeArchetype := !found || source == SourceArchetype + if includeAssessment || includeArchetype { + membership := assessment.NewMembershipResolver(h.DB(ctx)) + tagsResolver, err := assessment.NewTagResolver(h.DB(ctx)) + if err != nil { + _ = ctx.Error(err) + return + } + resolver := assessment.NewApplicationResolver(app, tagsResolver, membership, nil) + if includeArchetype { + archetypeTags, err := resolver.ArchetypeTags() + if err != nil { + _ = ctx.Error(err) + return + } + for i := range archetypeTags { + r := TagRef{} + r.With(archetypeTags[i].ID, archetypeTags[i].Name, SourceArchetype, true) + resources = append(resources, r) + } + } + if includeAssessment { + assessmentTags := resolver.AssessmentTags() + for i := range assessmentTags { + r := TagRef{} + r.With(assessmentTags[i].ID, assessmentTags[i].Name, SourceAssessment, true) + resources = append(resources, r) + } + } + } h.Respond(ctx, http.StatusOK, resources) } @@ -541,6 +584,11 @@ func (h ApplicationHandler) TagAdd(ctx *gin.Context) { _ = ctx.Error(err) return } + if ref.Virtual { + err = &BadRequestError{"cannot add virtual tags"} + _ = ctx.Error(err) + return + } app := &model.Application{} result := h.DB(ctx).First(app, id) if result.Error != nil { @@ -597,11 +645,13 @@ func (h ApplicationHandler) TagReplace(ctx *gin.Context) { if len(refs) > 0 { appTags := []model.ApplicationTag{} for _, ref := range refs { - appTags = append(appTags, model.ApplicationTag{ - ApplicationID: id, - TagID: ref.ID, - Source: source, - }) + if !ref.Virtual { + appTags = append(appTags, model.ApplicationTag{ + ApplicationID: id, + TagID: ref.ID, + Source: source, + }) + } } err = db.Create(&appTags).Error if err != nil { @@ -1089,7 +1139,7 @@ func (r *Application) With(m *model.Application, tags []model.ApplicationTag) { } for i := range tags { ref := TagRef{} - ref.With(tags[i].TagID, tags[i].Tag.Name, tags[i].Source) + ref.With(tags[i].TagID, tags[i].Tag.Name, tags[i].Source, false) r.Tags = append(r.Tags, ref) } r.Owner = r.refPtr(m.OwnerID, m.Owner) @@ -1121,11 +1171,11 @@ func (r *Application) WithArchetypes(archetypes []model.Archetype) { } // -// WithSourcedTags updates the resource with tags derived from assessments. -func (r *Application) WithSourcedTags(tags []model.Tag, source string) { +// WithVirtualTags updates the resource with tags derived from assessments. +func (r *Application) WithVirtualTags(tags []model.Tag, source string) { for _, t := range tags { ref := TagRef{} - ref.With(t.ID, t.Name, source) + ref.With(t.ID, t.Name, source, true) r.Tags = append(r.Tags, ref) } } diff --git a/api/base.go b/api/base.go index c6ee0fb43..260836f8b 100644 --- a/api/base.go +++ b/api/base.go @@ -370,17 +370,19 @@ func (r *Ref) With(id uint, name string) { // TagRef represents a reference to a Tag. // Contains the tag ID, name, tag source. type TagRef struct { - ID uint `json:"id" binding:"required"` - Name string `json:"name"` - Source string `json:"source"` + ID uint `json:"id" binding:"required"` + Name string `json:"name"` + Source string `json:"source"` + Virtual bool `json:"virtual,omitempty"` } // // With id and named model. -func (r *TagRef) With(id uint, name string, source string) { +func (r *TagRef) With(id uint, name string, source string, virtual bool) { r.ID = id r.Name = name r.Source = source + r.Virtual = virtual } // diff --git a/assessment/application.go b/assessment/application.go index 30406ab49..307cba291 100644 --- a/assessment/application.go +++ b/assessment/application.go @@ -8,9 +8,9 @@ import ( // NewApplicationResolver creates a new ApplicationResolver from an application and other shared resolvers. func NewApplicationResolver(app *model.Application, tags *TagResolver, membership *MembershipResolver, questionnaire *QuestionnaireResolver) (a *ApplicationResolver) { a = &ApplicationResolver{ - application: app, - tagResolver: tags, - membershipResolver: membership, + application: app, + tagResolver: tags, + membershipResolver: membership, questionnaireResolver: questionnaire, } return @@ -20,10 +20,10 @@ func NewApplicationResolver(app *model.Application, tags *TagResolver, membershi // ApplicationResolver wraps an Application model // with archetype and assessment resolution behavior. type ApplicationResolver struct { - application *model.Application - archetypes []model.Archetype - tagResolver *TagResolver - membershipResolver *MembershipResolver + application *model.Application + archetypes []model.Archetype + tagResolver *TagResolver + membershipResolver *MembershipResolver questionnaireResolver *QuestionnaireResolver } diff --git a/assessment/assessment_test.go b/assessment/assessment_test.go index 7ae3e825b..f63c7d537 100644 --- a/assessment/assessment_test.go +++ b/assessment/assessment_test.go @@ -89,4 +89,4 @@ func TestPrepareSections(t *testing.T) { g.Expect(questions[2].Answers[0].Text).To(gomega.Equal("Answer1")) g.Expect(questions[2].Answers[0].AutoAnswered).To(gomega.BeTrue()) g.Expect(questions[2].Answers[0].Selected).To(gomega.BeTrue()) -} \ No newline at end of file +} diff --git a/assessment/membership.go b/assessment/membership.go index 1ec0dde74..e6eeee83a 100644 --- a/assessment/membership.go +++ b/assessment/membership.go @@ -19,11 +19,11 @@ func NewMembershipResolver(db *gorm.DB) (m *MembershipResolver) { // // MembershipResolver resolves archetype membership. type MembershipResolver struct { - db *gorm.DB - archetypes []model.Archetype - tagSets map[uint]Set + db *gorm.DB + archetypes []model.Archetype + tagSets map[uint]Set archetypeMembers map[uint][]model.Application - membersCached bool + membersCached bool } // @@ -61,9 +61,9 @@ func (r *MembershipResolver) Archetypes(m *model.Application) (archetypes []mode // throw away any archetypes that are a subset of // another archetype-- only keep the most specific matches. - loop: +loop: for _, a1 := range matches { - for _, a2 := range matches{ + for _, a2 := range matches { if a1.ID == a2.ID { continue } diff --git a/assessment/pkg.go b/assessment/pkg.go index dadb144bc..c1c70238d 100644 --- a/assessment/pkg.go +++ b/assessment/pkg.go @@ -10,31 +10,31 @@ import ( // Assessment risk const ( RiskUnknown = "unknown" - RiskRed = "red" - RiskYellow = "yellow" - RiskGreen = "green" + RiskRed = "red" + RiskYellow = "yellow" + RiskGreen = "green" ) // // Confidence adjustment const ( - AdjusterRed = 0.5 + AdjusterRed = 0.5 AdjusterYellow = 0.98 ) // // Confidence multiplier. const ( - MultiplierRed = 0.6 + MultiplierRed = 0.6 MultiplierYellow = 0.95 ) // // Risk weights const ( - WeightRed = 1 - WeightYellow = 80 - WeightGreen = 100 + WeightRed = 1 + WeightYellow = 80 + WeightGreen = 100 WeightUnknown = 70 ) diff --git a/assessment/questionnaire.go b/assessment/questionnaire.go index eecbb6f7a..d1fa86d01 100644 --- a/assessment/questionnaire.go +++ b/assessment/questionnaire.go @@ -19,7 +19,7 @@ func NewQuestionnaireResolver(db *gorm.DB) (a *QuestionnaireResolver, err error) // // QuestionnaireResolver resolves questionnaire logic. type QuestionnaireResolver struct { - db *gorm.DB + db *gorm.DB requiredQuestionnaires Set } @@ -47,7 +47,7 @@ func (r *QuestionnaireResolver) cacheQuestionnaires() (err error) { // questionnaires. func (r *QuestionnaireResolver) Assessed(assessments []model.Assessment) (assessed bool) { answered := NewSet() - loop: +loop: for _, a := range assessments { if r.requiredQuestionnaires.Contains(a.QuestionnaireID) { sections := []Section{} @@ -63,4 +63,4 @@ func (r *QuestionnaireResolver) Assessed(assessments []model.Assessment) (assess assessed = answered.Superset(r.requiredQuestionnaires) return -} \ No newline at end of file +} diff --git a/assessment/section.go b/assessment/section.go index 18bd22c5e..471eefe36 100644 --- a/assessment/section.go +++ b/assessment/section.go @@ -132,4 +132,3 @@ type Thresholds struct { Yellow uint `json:"yellow" yaml:"yellow"` Unknown uint `json:"unknown" yaml:"unknown"` } - diff --git a/assessment/tag.go b/assessment/tag.go index aed421f88..25c8ee55d 100644 --- a/assessment/tag.go +++ b/assessment/tag.go @@ -22,7 +22,7 @@ func NewTagResolver(db *gorm.DB) (t *TagResolver, err error) { // TagResolver resolves CategorizedTags to Tag models. type TagResolver struct { cache map[string]map[string]*model.Tag - db *gorm.DB + db *gorm.DB } // @@ -66,4 +66,4 @@ func (r *TagResolver) cacheTags() (err error) { } return -} \ No newline at end of file +} diff --git a/metrics/manager.go b/metrics/manager.go index ba1723e81..292a1db3a 100644 --- a/metrics/manager.go +++ b/metrics/manager.go @@ -44,4 +44,4 @@ func (m *Manager) gaugeApplications() { Log.Error(result.Error, "unable to gauge applications") } Applications.Set(float64(count)) -} \ No newline at end of file +} diff --git a/metrics/pkg.go b/metrics/pkg.go index 40e60fc10..9160a9563 100644 --- a/metrics/pkg.go +++ b/metrics/pkg.go @@ -22,4 +22,4 @@ var ( Name: "konveyor_issues_exported_total", Help: "The total number of issues exported to external trackers", }) -) \ No newline at end of file +) From 2ff6621ecacd71ded0f88744de3859ead857d612 Mon Sep 17 00:00:00 2001 From: Sam Lucidi Date: Fri, 29 Sep 2023 13:39:10 -0400 Subject: [PATCH 2/2] Use Source* consts as intended Signed-off-by: Sam Lucidi --- api/application.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/application.go b/api/application.go index 6fba7581f..833383885 100644 --- a/api/application.go +++ b/api/application.go @@ -147,8 +147,8 @@ func (h ApplicationHandler) Get(ctx *gin.Context) { r := Application{} r.With(m, tags) r.WithArchetypes(archetypes) - r.WithVirtualTags(archetypeTags, "archetype") - r.WithVirtualTags(resolver.AssessmentTags(), "assessment") + r.WithVirtualTags(archetypeTags, SourceArchetype) + r.WithVirtualTags(resolver.AssessmentTags(), SourceAssessment) r.Assessed, err = resolver.Assessed() if err != nil { _ = ctx.Error(err) @@ -210,8 +210,8 @@ func (h ApplicationHandler) List(ctx *gin.Context) { r := Application{} r.With(&list[i], tags) r.WithArchetypes(archetypes) - r.WithVirtualTags(archetypeTags, "archetype") - r.WithVirtualTags(resolver.AssessmentTags(), "assessment") + r.WithVirtualTags(archetypeTags, SourceArchetype) + r.WithVirtualTags(resolver.AssessmentTags(), SourceAssessment) r.Assessed, err = resolver.Assessed() if err != nil { _ = ctx.Error(err) @@ -286,8 +286,8 @@ func (h ApplicationHandler) Create(ctx *gin.Context) { r.With(m, tags) r.WithArchetypes(archetypes) - r.WithVirtualTags(archetypeTags, "archetype") - r.WithVirtualTags(resolver.AssessmentTags(), "assessment") + r.WithVirtualTags(archetypeTags, SourceArchetype) + r.WithVirtualTags(resolver.AssessmentTags(), SourceArchetype) r.Assessed, err = resolver.Assessed() if err != nil { _ = ctx.Error(err)