Skip to content

Commit 534d7a4

Browse files
committed
fix: resolve NULL email scan in comments and merge patch for owners/labels
Fix #214: Comment queries scanned nullable user fields (email, uuid) into plain Go strings, which fails when the value is NULL. Added a CommentModel with sql.NullString fields (matching the existing DiscussionModel pattern) to safely handle nullable columns. Fix #173: UpsertPatchAsset deep-merged the Data map but completely replaced Owners (slice) and Labels (map). Now all three fields merge consistently — existing labels are preserved with new keys added, and existing owners are kept with new owners appended (deduplicated by email or uuid).
1 parent 0506466 commit 534d7a4

File tree

4 files changed

+114
-9
lines changed

4 files changed

+114
-9
lines changed

core/asset/asset_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,10 +310,12 @@ func TestAssetPatch(t *testing.T) {
310310
Name: "new-name",
311311
URL: "https://sample-url.com",
312312
Labels: map[string]string{
313+
"foo": "bar",
313314
"bar": "foo",
314315
"bar2": "foo2",
315316
},
316317
Owners: []user.User{
318+
{Email: "old@example.com"},
317319
{Email: "new@example.com"},
318320
{Email: "new2@example.com"},
319321
},
@@ -359,10 +361,12 @@ func TestAssetPatch(t *testing.T) {
359361
Name: "new-name",
360362
URL: "https://sample-url.com",
361363
Labels: map[string]string{
364+
"foo": "bar",
362365
"bar": "foo",
363366
"bar2": "foo2",
364367
},
365368
Owners: []user.User{
369+
{Email: "old@example.com"},
366370
{Email: "new@example.com"},
367371
{Email: "new2@example.com"},
368372
},
@@ -397,6 +401,12 @@ func TestAssetPatch(t *testing.T) {
397401
Service: "firehose",
398402
Description: "new-description",
399403
Name: "new-name",
404+
Labels: map[string]string{
405+
"foo": "bar",
406+
},
407+
Owners: []user.User{
408+
{Email: "old@example.com"},
409+
},
400410
},
401411
},
402412
{

core/asset/patch.go

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package asset
22

33
import (
4-
"github.com/raystack/compass/core/user"
54
"github.com/peterbourgon/mergemap"
5+
"github.com/raystack/compass/core/user"
66
)
77

88
// patch appends asset with data from map. It mutates the asset itself.
@@ -16,18 +16,80 @@ func patchAsset(a *Asset, patchData map[string]interface{}) {
1616

1717
labels, exists := patchData["labels"]
1818
if exists {
19-
a.Labels = buildLabels(labels)
19+
a.Labels = mergeLabels(a.Labels, buildLabels(labels))
2020
}
2121
owners, exists := patchData["owners"]
2222
if exists {
23-
a.Owners = buildOwners(owners)
23+
a.Owners = mergeOwners(a.Owners, buildOwners(owners))
2424
}
2525
data, exists := patchData["data"]
2626
if exists {
2727
patchAssetData(a, data)
2828
}
2929
}
3030

31+
// mergeLabels merges new labels into existing ones. New values override existing keys.
32+
func mergeLabels(existing, patch map[string]string) map[string]string {
33+
if existing == nil {
34+
return patch
35+
}
36+
if patch == nil {
37+
return existing
38+
}
39+
for k, v := range patch {
40+
existing[k] = v
41+
}
42+
return existing
43+
}
44+
45+
// mergeOwners merges new owners into existing ones, deduplicating by email or uuid.
46+
func mergeOwners(existing, patch []user.User) []user.User {
47+
if existing == nil {
48+
return patch
49+
}
50+
if patch == nil {
51+
return existing
52+
}
53+
54+
seen := make(map[string]int) // key -> index in result
55+
result := make([]user.User, len(existing))
56+
copy(result, existing)
57+
58+
for i, o := range result {
59+
if key := ownerKey(o); key != "" {
60+
seen[key] = i
61+
}
62+
}
63+
64+
for _, o := range patch {
65+
key := ownerKey(o)
66+
if key == "" {
67+
result = append(result, o)
68+
continue
69+
}
70+
if idx, exists := seen[key]; exists {
71+
// update existing owner in-place
72+
result[idx] = o
73+
} else {
74+
seen[key] = len(result)
75+
result = append(result, o)
76+
}
77+
}
78+
79+
return result
80+
}
81+
82+
// ownerKey returns a deduplication key for an owner (email preferred, then uuid).
83+
func ownerKey(o user.User) string {
84+
if o.Email != "" {
85+
return "email:" + o.Email
86+
}
87+
if o.UUID != "" {
88+
return "uuid:" + o.UUID
89+
}
90+
return ""
91+
}
92+
3193
// buildLabels builds labels from interface{}
3294
func buildLabels(data interface{}) (labels map[string]string) {
3395
switch d := data.(type) {

store/postgres/comment_model.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package postgres
2+
3+
import (
4+
"time"
5+
6+
"github.com/raystack/compass/core/discussion"
7+
)
8+
9+
type CommentModel struct {
10+
ID string `db:"id"`
11+
DiscussionID string `db:"discussion_id"`
12+
Body string `db:"body"`
13+
Owner UserModel `db:"owner"`
14+
UpdatedBy UserModel `db:"updated_by"`
15+
CreatedAt time.Time `db:"created_at"`
16+
UpdatedAt time.Time `db:"updated_at"`
17+
}
18+
19+
func (cm CommentModel) toComment() discussion.Comment {
20+
return discussion.Comment{
21+
ID: cm.ID,
22+
DiscussionID: cm.DiscussionID,
23+
Body: cm.Body,
24+
Owner: cm.Owner.toUser(),
25+
UpdatedBy: cm.UpdatedBy.toUser(),
26+
CreatedAt: cm.CreatedAt,
27+
UpdatedAt: cm.UpdatedAt,
28+
}
29+
}

store/postgres/discussion_comment_repository.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,16 @@ func (r *DiscussionRepository) GetAllComments(ctx context.Context, did string, f
6060
return nil, fmt.Errorf("error building query: %w", err)
6161
}
6262

63-
cmts := []discussion.Comment{}
64-
err = r.client.SelectContext(ctx, &cmts, query, args...)
63+
var models []CommentModel
64+
err = r.client.SelectContext(ctx, &models, query, args...)
6565
if err != nil {
6666
return nil, fmt.Errorf("error getting list of comments: %w", err)
6767
}
6868

69+
cmts := make([]discussion.Comment, 0, len(models))
70+
for _, m := range models {
71+
cmts = append(cmts, m.toComment())
72+
}
6973
return cmts, nil
7074
}
7175

@@ -82,16 +86,16 @@ func (r *DiscussionRepository) GetComment(ctx context.Context, cid string, did s
8286
return discussion.Comment{}, fmt.Errorf("error building query: %w", err)
8387
}
8488

85-
cmt := discussion.Comment{}
86-
err = r.client.GetContext(ctx, &cmt, query, args...)
89+
var model CommentModel
90+
err = r.client.GetContext(ctx, &model, query, args...)
8791
if errors.Is(err, sql.ErrNoRows) {
8892
return discussion.Comment{}, discussion.NotFoundError{CommentID: cid, DiscussionID: did}
8993
}
9094
if err != nil {
91-
return discussion.Comment{}, fmt.Errorf("error getting list of comments: %w", err)
95+
return discussion.Comment{}, fmt.Errorf("error getting comment: %w", err)
9296
}
9397

94-
return cmt, nil
98+
return model.toComment(), nil
9599
}
96100

97101
// Update updates a comment

0 commit comments

Comments
 (0)