Skip to content

Commit

Permalink
🐛 Exclude virtual tags from writing/updating (#495)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
mansam authored Sep 29, 2023
1 parent bb3d5f0 commit f1da666
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 54 deletions.
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ HUB_BASE_URL ?= http://localhost:8080

PKG = ./addon/... \
./api/... \
./assessment/... \
./auth/... \
./cmd/... \
./database/... \
./encryption/... \
./importer/... \
./k8s/... \
./metrics/... \
./migration/... \
./model/... \
./settings/... \
Expand Down
90 changes: 70 additions & 20 deletions api/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ const (
Source = "source"
)

//
// Tag Sources
const (
SourceAssessment = "assessment"
SourceArchetype = "archetype"
)

//
// ApplicationHandler handles application resource routes.
type ApplicationHandler struct {
Expand Down Expand Up @@ -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, SourceArchetype)
r.WithVirtualTags(resolver.AssessmentTags(), SourceAssessment)
r.Assessed, err = resolver.Assessed()
if err != nil {
_ = ctx.Error(err)
Expand Down Expand Up @@ -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, SourceArchetype)
r.WithVirtualTags(resolver.AssessmentTags(), SourceAssessment)
r.Assessed, err = resolver.Assessed()
if err != nil {
_ = ctx.Error(err)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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, SourceArchetype)
r.WithVirtualTags(resolver.AssessmentTags(), SourceArchetype)
r.Assessed, err = resolver.Assessed()
if err != nil {
_ = ctx.Error(err)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand Down
10 changes: 6 additions & 4 deletions api/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

//
Expand Down
14 changes: 7 additions & 7 deletions assessment/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion assessment/assessment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
12 changes: 6 additions & 6 deletions assessment/membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

//
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions assessment/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
6 changes: 3 additions & 3 deletions assessment/questionnaire.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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{}
Expand All @@ -63,4 +63,4 @@ func (r *QuestionnaireResolver) Assessed(assessments []model.Assessment) (assess
assessed = answered.Superset(r.requiredQuestionnaires)

return
}
}
1 change: 0 additions & 1 deletion assessment/section.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,3 @@ type Thresholds struct {
Yellow uint `json:"yellow" yaml:"yellow"`
Unknown uint `json:"unknown" yaml:"unknown"`
}

4 changes: 2 additions & 2 deletions assessment/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

//
Expand Down Expand Up @@ -66,4 +66,4 @@ func (r *TagResolver) cacheTags() (err error) {
}

return
}
}
2 changes: 1 addition & 1 deletion metrics/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,4 @@ func (m *Manager) gaugeApplications() {
Log.Error(result.Error, "unable to gauge applications")
}
Applications.Set(float64(count))
}
}
2 changes: 1 addition & 1 deletion metrics/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ var (
Name: "konveyor_issues_exported_total",
Help: "The total number of issues exported to external trackers",
})
)
)

0 comments on commit f1da666

Please sign in to comment.