Skip to content

Commit

Permalink
Ban transformer: Take three on fixing out of bounds
Browse files Browse the repository at this point in the history
We were modifying the array we were iterating over, which caused some
rather interesting bugs. Take a copy instead.

We also trim the array at the end to leave a tidier end result.
  • Loading branch information
KristianLyng committed Dec 1, 2023
1 parent 5564ff7 commit eff2dc2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 24 deletions.
31 changes: 14 additions & 17 deletions transformer/ban.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,34 @@ type Ban struct {
}

func (b *Ban) Transform(c *skogul.Container) error {

for pathKey, pathValue := range b.LookupData {
for k, mi := range c.Metrics {
newMetrics := make([]*skogul.Metric, 0, len(c.Metrics))
for _, mi := range c.Metrics {
var ptr interface{}

ptr, _ = jsonptr.Get(mi.Data, pathKey)

if ptr == pathValue {
if k == len(c.Metrics)-1 {
c.Metrics = c.Metrics[:k]
} else {
c.Metrics = append(c.Metrics[:k], c.Metrics[k+1:]...)
}
if ptr != pathValue {
newMetrics = append(newMetrics, mi)
}
}
c.Metrics = newMetrics
}

for pathKey, pathValue := range b.LookupMetadata {
for k, mi := range c.Metrics {
newMetrics := make([]*skogul.Metric, 0, len(c.Metrics))
for _, mi := range c.Metrics {
var ptr interface{}

ptr, _ = jsonptr.Get(mi.Metadata, pathKey)
if ptr == pathValue {
if k == len(c.Metrics)-1 {
c.Metrics = c.Metrics[:k]
} else {
c.Metrics = append(c.Metrics[:k], c.Metrics[k+1:]...)
}
continue
if ptr != pathValue {
newMetrics = append(newMetrics, mi)
}
}
c.Metrics = newMetrics
}

newMetrics := make([]*skogul.Metric, len(c.Metrics))
copy(newMetrics, c.Metrics)
c.Metrics = newMetrics
return nil
}
24 changes: 17 additions & 7 deletions transformer/ban_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,20 @@ func TestBan(t *testing.T) {
"funny": "notfunny",
},
}
metric5 := skogul.Metric{
Metadata: map[string]interface{}{
"funny": "",
},
}
metric6 := skogul.Metric{
Metadata: map[string]interface{}{
"funny": "",
},
}

c := skogul.Container{}
c.Metrics = []*skogul.Metric{&metric, &metric2, &metric3, &metric4}
c.Metrics = []*skogul.Metric{&metric, &metric2, &metric3, &metric4, &metric5, &metric6}

originalMetricsCount := len(c.Metrics)
ban := &transformer.Ban{}

ban.LookupData = map[string]interface{}{
Expand All @@ -113,16 +122,17 @@ func TestBan(t *testing.T) {
}

ban.LookupMetadata = map[string]interface{}{
"/funny": "notfunny",
"/funny": "",
}

err := ban.Transform(&c)
if err != nil {
t.Fatalf("error occurred %v", err.Error())
}

if originalMetricsCount == len(c.Metrics) {
t.Fatal("Metrics slice has same length even after the banning of values")
if len(c.Metrics) != 1 {
t.Fatalf("expected exactly 1 metric to remain, got %d", len(c.Metrics))
}
if cap(c.Metrics) != 1 {
t.Fatalf("expected exactly len(metrics) == 1, got %d", cap(c.Metrics))
}

}

0 comments on commit eff2dc2

Please sign in to comment.