Skip to content

Commit 5a0883d

Browse files
authored
Add pcommon.Map helper to add a key to the map if does not exists (#14023)
This new API is intended to be similar with "LoadOrStore" from the `sync.Map` and a helper to avoid iterating over the map (lookup) twice for when we need to only insert a key if it does not exists. Example https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/coreinternal/attraction/attraction.go#L307: ``` if _, found = attrs.Get(action.Key); found { continue } av.CopyTo(attrs.PutEmpty(action.Key)) ``` We lookup key in `Get` as well as `PutEmpty`, and this can be re-written as: ``` if val, found = attrs.GetOrPutEmpty(action.Key); !found { av.CopyTo(val) } ``` Signed-off-by: Bogdan Drutu <[email protected]>
1 parent c9aacb2 commit 5a0883d

File tree

3 files changed

+58
-0
lines changed

3 files changed

+58
-0
lines changed

.chloggen/get-or-put-empty.yaml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: pkg/pdata
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Add pcommon.Map helper to add a key to the map if does not exists
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [14023]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [api]

pdata/pcommon/map.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ func (m Map) PutEmpty(k string) Value {
121121
return newValue(&(*m.getOrig())[len(*m.getOrig())-1].Value, m.getState())
122122
}
123123

124+
// GetOrPutEmpty returns the Value associated with the key and true (loaded) if the key exists in the map,
125+
// otherwise inserts an empty value to the map under the given key and returns the inserted value
126+
// and false (loaded).
127+
func (m Map) GetOrPutEmpty(k string) (Value, bool) {
128+
m.getState().AssertMutable()
129+
if av, existing := m.Get(k); existing {
130+
return av, true
131+
}
132+
*m.getOrig() = append(*m.getOrig(), otlpcommon.KeyValue{Key: k})
133+
return newValue(&(*m.getOrig())[len(*m.getOrig())-1].Value, m.getState()), false
134+
}
135+
124136
// PutStr performs the Insert or Update action. The Value is
125137
// inserted to the map that did not originally have the key. The key/value is
126138
// updated to the map where the key already existed.

pdata/pcommon/map_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ func TestMapReadOnly(t *testing.T) {
7676
assert.Panics(t, func() { m.PutInt("k2", 123) })
7777
assert.Panics(t, func() { m.PutDouble("k2", 1.23) })
7878
assert.Panics(t, func() { m.PutBool("k2", true) })
79+
assert.Panics(t, func() { m.PutEmpty("foo") })
80+
assert.Panics(t, func() { m.GetOrPutEmpty("foo") })
7981
assert.Panics(t, func() { m.PutEmptyBytes("k2") })
8082
assert.Panics(t, func() { m.PutEmptyMap("k2") })
8183
assert.Panics(t, func() { m.PutEmptySlice("k2") })
@@ -111,6 +113,24 @@ func TestMapPutEmpty(t *testing.T) {
111113
assert.Equal(t, int64(1), v2.Int())
112114
}
113115

116+
func TestMapGetOrPutEmpty(t *testing.T) {
117+
m := NewMap()
118+
v := m.PutEmpty("k1")
119+
v.SetStr("test")
120+
assert.Equal(t, map[string]any{
121+
"k1": "test",
122+
}, m.AsRaw())
123+
124+
v, found := m.GetOrPutEmpty("k1")
125+
assert.True(t, found)
126+
require.Equal(t, ValueTypeStr, v.Type())
127+
assert.Equal(t, "test", v.Str())
128+
129+
v, found = m.GetOrPutEmpty("k2")
130+
assert.False(t, found)
131+
require.Equal(t, ValueTypeEmpty, v.Type())
132+
}
133+
114134
func TestMapPutEmptyMap(t *testing.T) {
115135
m := NewMap()
116136
childMap := m.PutEmptyMap("k1")
@@ -607,6 +627,7 @@ func TestInvalidMap(t *testing.T) {
607627
assert.Panics(t, func() { v.Remove("foo") })
608628
assert.Panics(t, func() { v.RemoveIf(testFunc) })
609629
assert.Panics(t, func() { v.PutEmpty("foo") })
630+
assert.Panics(t, func() { v.GetOrPutEmpty("foo") })
610631
assert.Panics(t, func() { v.PutStr("foo", "bar") })
611632
assert.Panics(t, func() { v.PutInt("foo", 1) })
612633
assert.Panics(t, func() { v.PutDouble("foo", 1.1) })

0 commit comments

Comments
 (0)