Skip to content

Commit 4345068

Browse files
committed
Group same-field relations by group ID, not label.
1 parent 1b4e20b commit 4345068

12 files changed

Lines changed: 473 additions & 68 deletions

api/handle_graph_walk.go

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import (
55
"strconv"
66
"strings"
77

8+
"github.com/cockroachdb/errors"
89
"github.com/gin-gonic/gin"
10+
"github.com/google/uuid"
911

1012
"github.com/checkmarble/marble-backend/dto"
1113
"github.com/checkmarble/marble-backend/models"
@@ -25,18 +27,16 @@ func handleGraphWalk(uc usecases.Usecases) func(c *gin.Context) {
2527
nodeType := c.Param("node_type")
2628
nodeId := c.Param("node_id")
2729

30+
relationGroupIds, err := parseGraphRelationGroups(c.Query("same_field_relations"))
31+
if presentError(ctx, c, err) {
32+
return
33+
}
34+
2835
opts := models.GraphWalkOptions{
2936
EndTypes: parseGraphEndTypes(c.Query("types")),
3037
Degrees: parseGraphDegrees(c.Query("degrees")),
3138
SkipSameFieldRelations: c.Query("skip_same_field_relations") == "true",
32-
SameFieldRelations: func() []string {
33-
switch sfr := c.Query("same_field_relations"); sfr {
34-
case "":
35-
return nil
36-
default:
37-
return strings.Split(sfr, ",")
38-
}
39-
}(),
39+
SameFieldRelations: relationGroupIds,
4040
}
4141

4242
usecase := usecasesWithCreds(ctx, uc).NewGraphWalkUsecase()
@@ -50,6 +50,29 @@ func handleGraphWalk(uc usecases.Usecases) func(c *gin.Context) {
5050
}
5151
}
5252

53+
// parseGraphRelationGroups reads the relation groups a walk should restrict its same-field
54+
// traversal to, as a comma-separated list of group ids. Empty means "every group".
55+
func parseGraphRelationGroups(raw string) ([]uuid.UUID, error) {
56+
var groupIds []uuid.UUID
57+
58+
for part := range strings.SplitSeq(raw, ",") {
59+
trimmed := strings.TrimSpace(part)
60+
if trimmed == "" {
61+
continue
62+
}
63+
64+
groupId, err := uuid.Parse(trimmed)
65+
if err != nil {
66+
return nil, errors.Wrapf(models.BadParameterError,
67+
"%q is not a valid relation group id", trimmed)
68+
}
69+
70+
groupIds = append(groupIds, groupId)
71+
}
72+
73+
return groupIds, nil
74+
}
75+
5376
func parseGraphEndTypes(raw string) []string {
5477
var endTypes []string
5578

api/handle_graph_walk_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package api
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/uuid"
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/checkmarble/marble-backend/models"
11+
)
12+
13+
func TestParseGraphRelationGroups(t *testing.T) {
14+
first := uuid.New()
15+
second := uuid.New()
16+
17+
tests := []struct {
18+
name string
19+
raw string
20+
expected []uuid.UUID
21+
}{
22+
{name: "absent means every group", raw: "", expected: nil},
23+
{name: "one group", raw: first.String(), expected: []uuid.UUID{first}},
24+
{
25+
name: "several groups",
26+
raw: first.String() + "," + second.String(),
27+
expected: []uuid.UUID{first, second},
28+
},
29+
{
30+
// A list is easier to build with a trailing comma or a space after each one, and
31+
// neither says anything different from the list without them.
32+
name: "padding and empty segments are not an error",
33+
raw: " " + first.String() + ", " + second.String() + ",",
34+
expected: []uuid.UUID{first, second},
35+
},
36+
}
37+
38+
for _, tt := range tests {
39+
t.Run(tt.name, func(t *testing.T) {
40+
groupIds, err := parseGraphRelationGroups(tt.raw)
41+
42+
require.NoError(t, err)
43+
assert.Equal(t, tt.expected, groupIds)
44+
})
45+
}
46+
}
47+
48+
func TestParseGraphRelationGroups_ReportsSomethingThatIsNotAGroupIdAsBadInput(t *testing.T) {
49+
// Bad input from a query string is the caller's mistake, not ours: it must not read as an
50+
// internal failure, which is what would be reported and alerted on otherwise.
51+
_, err := parseGraphRelationGroups("same_iban")
52+
53+
assert.ErrorIs(t, err, models.BadParameterError)
54+
}

dto/graph_relation_dto.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
type GraphRelation struct {
1212
Id uuid.UUID `json:"id"`
13+
GroupId uuid.UUID `json:"group_id"`
1314
Label string `json:"label"`
1415
LeftType string `json:"left_type"`
1516
LeftField string `json:"left_field"`
@@ -21,6 +22,7 @@ type GraphRelation struct {
2122
func AdaptGraphRelationDto(r models.GraphRelation) GraphRelation {
2223
return GraphRelation{
2324
Id: r.Id,
25+
GroupId: r.GroupId,
2426
Label: r.Label,
2527
LeftType: r.LeftType,
2628
LeftField: r.LeftField,
@@ -31,15 +33,17 @@ func AdaptGraphRelationDto(r models.GraphRelation) GraphRelation {
3133
}
3234

3335
type CreateGraphRelationBody struct {
34-
Label string `json:"label" binding:"required"`
35-
LeftType string `json:"left_type" binding:"required"`
36-
LeftField string `json:"left_field" binding:"required"`
37-
RightType string `json:"right_type" binding:"required"`
38-
RightField string `json:"right_field" binding:"required"`
36+
GroupId uuid.UUID `json:"group_id"`
37+
Label string `json:"label" binding:"required"`
38+
LeftType string `json:"left_type" binding:"required"`
39+
LeftField string `json:"left_field" binding:"required"`
40+
RightType string `json:"right_type" binding:"required"`
41+
RightField string `json:"right_field" binding:"required"`
3942
}
4043

4144
func AdaptCreateGraphRelation(body CreateGraphRelationBody) models.CreateGraphRelation {
4245
return models.CreateGraphRelation{
46+
GroupId: body.GroupId,
4347
Label: body.Label,
4448
LeftType: body.LeftType,
4549
LeftField: body.LeftField,

mocks/graph_relation_repository.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ func (r *GraphRelationRepository) ListGraphRelations(
2121
return args.Get(0).([]models.GraphRelation), args.Error(1)
2222
}
2323

24+
func (r *GraphRelationRepository) GetGraphRelationGroupLabel(ctx context.Context, exec repositories.Executor, orgId, groupId uuid.UUID) (string, error) {
25+
args := r.Called(ctx, exec, orgId, groupId)
26+
return args.Get(0).(string), args.Error(1)
27+
}
28+
2429
func (r *GraphRelationRepository) GetGraphRelation(
2530
ctx context.Context, exec repositories.Executor, id uuid.UUID,
2631
) (models.GraphRelation, error) {

models/graph_walk.go

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,9 @@ type GraphResultNode struct {
7474
// the value it pivots on.
7575
//
7676
// - "match": a value two or more records share on a field an organization declared as
77-
// meaningful. Type is the relation's label, Id the shared value. Its edges are the
78-
// records carrying it, so a value shared by n records costs n edges rather than n².
77+
// meaningful. Type is the relation's group id, Id the shared value, so relations sharing
78+
// a group converge on one connector whatever each is labelled. Its edges are the records
79+
// carrying it, so a value shared by n records costs n edges rather than n².
7980
//
8081
// - "link": the records hanging off one record through a single data-model link, when
8182
// there are too many of them to pull in. Type is the link's name, Id the value the
@@ -94,12 +95,23 @@ type GraphResultNode struct {
9495
Metadata GraphResultNodeMetadata
9596
}
9697

98+
// GraphResultNodeMetadata is what is known about a node beyond its identity: the label to show
99+
// it under, and — for a record — the risk it carries.
97100
type GraphResultNodeMetadata struct {
98101
Index int
99-
// Label is the record's caption: the value it carries on the field its table declares as
100-
// its caption field. It is empty on a connector, which is not a record, and on a record
101-
// whose table declares no caption field.
102-
Label string
102+
103+
// Label is what to show the node under, and where it comes from depends on what the node is:
104+
//
105+
// - on a record, its caption: the value it carries on the field its table declares as its
106+
// caption field. Empty when its table declares no such field.
107+
// - on a connector, what to call the relationship: the relation group's label for a "match"
108+
// one, the link's name for a "link" one. There it is deliberately not an identity — two
109+
// independent groups may well be labelled the same — so Type still carries the group id,
110+
// and Type/Id is what edges refer to.
111+
Label string
112+
113+
// RiskLevel and Tags come from the records' own scoring, so they are only ever set on a
114+
// record node: a connector is not a record and has nothing to score.
103115
RiskLevel int
104116
Tags []uuid.UUID
105117
}
@@ -122,19 +134,22 @@ type GraphWalkOptions struct {
122134
EndTypes []string
123135
Degrees int
124136
SkipSameFieldRelations bool
125-
SameFieldRelations []string
137+
SameFieldRelations []uuid.UUID
126138
}
127139

128140
// GraphRelation declares that equal values of two (record type, field) endpoints connect the
129141
// records carrying them, even though no link exists between those records. An organization
130142
// defines its own relations against the tables and fields of its own data model.
131143
//
132144
// Relations are one-to-one: a group of three endpoints that should all count as sharing a
133-
// value is expressed as three relations (A<->B, B<->C, C<->A). Relations sharing a Label
134-
// converge on the same connector node, so such a group still renders as a single star.
145+
// value is expressed as three relations (A<->B, B<->C, C<->A). Relations sharing a GroupId
146+
// converge on the same connector node, so such a group still renders as a single star. The
147+
// creation path keeps Label consistent across a group, but GroupId — not Label — is what
148+
// identifies it: unlike a label, it survives a rename without splitting or merging groups.
135149
type GraphRelation struct {
136150
Id uuid.UUID
137151
OrgId uuid.UUID
152+
GroupId uuid.UUID
138153
Label string
139154
LeftType string
140155
LeftField string
@@ -263,6 +278,7 @@ func GraphIndexedFields(dataModel DataModel, relations []GraphRelation) map[stri
263278
// database.
264279
type CreateGraphRelation struct {
265280
OrgId uuid.UUID
281+
GroupId uuid.UUID
266282
Label string
267283
LeftType string
268284
LeftField string

repositories/dbmodels/db_graph_relation.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ var SelectGraphRelationColumn = utils.ColumnList[DBGraphRelation]()
1616
type DBGraphRelation struct {
1717
Id uuid.UUID `db:"id"`
1818
OrgId uuid.UUID `db:"org_id"`
19+
GroupId uuid.UUID `db:"group_id"`
1920
Label string `db:"label"`
2021
LeftType string `db:"left_type"`
2122
LeftField string `db:"left_field"`
@@ -28,6 +29,7 @@ func AdaptGraphRelation(db DBGraphRelation) (models.GraphRelation, error) {
2829
return models.GraphRelation{
2930
Id: db.Id,
3031
OrgId: db.OrgId,
32+
GroupId: db.GroupId,
3133
Label: db.Label,
3234
LeftType: db.LeftType,
3335
LeftField: db.LeftField,

repositories/graph_relation_repository.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66
"strings"
77

88
"github.com/Masterminds/squirrel"
9+
"github.com/cockroachdb/errors"
910
"github.com/google/uuid"
11+
"github.com/jackc/pgx/v5"
1012

1113
"github.com/checkmarble/marble-backend/models"
1214
"github.com/checkmarble/marble-backend/pure_utils"
@@ -15,6 +17,7 @@ import (
1517

1618
type GraphRelationRepository interface {
1719
ListGraphRelations(ctx context.Context, exec Executor, orgId uuid.UUID) ([]models.GraphRelation, error)
20+
GetGraphRelationGroupLabel(ctx context.Context, exec Executor, orgId, groupId uuid.UUID) (string, error)
1821
GetGraphRelation(ctx context.Context, exec Executor, id uuid.UUID) (models.GraphRelation, error)
1922
CreateGraphRelation(ctx context.Context, exec Executor, relation models.CreateGraphRelation) (models.GraphRelation, error)
2023
DeleteGraphRelation(ctx context.Context, exec Executor, id uuid.UUID) error
@@ -38,6 +41,37 @@ func (repo *MarbleDbRepository) ListGraphRelations(
3841
return SqlToListOfModels(ctx, exec, query, dbmodels.AdaptGraphRelation)
3942
}
4043

44+
func (repo *MarbleDbRepository) GetGraphRelationGroupLabel(ctx context.Context, exec Executor, orgId, groupId uuid.UUID) (string, error) {
45+
if err := validateMarbleDbExecutor(exec); err != nil {
46+
return "", err
47+
}
48+
49+
sql := NewQueryBuilder().
50+
Select("label").
51+
From(dbmodels.TABLE_GRAPH_RELATIONS).
52+
Where(squirrel.Eq{"org_id": orgId, "group_id": groupId}).
53+
Limit(1)
54+
55+
query, args, err := sql.ToSql()
56+
if err != nil {
57+
return "", err
58+
}
59+
60+
row := exec.QueryRow(ctx, query, args...)
61+
62+
var label string
63+
64+
if err := row.Scan(&label); err != nil {
65+
if errors.Is(err, pgx.ErrNoRows) {
66+
return "", errors.Wrap(models.NotFoundError, "provided group does not exist")
67+
}
68+
69+
return "", err
70+
}
71+
72+
return label, nil
73+
}
74+
4175
func (repo *MarbleDbRepository) GetGraphRelation(
4276
ctx context.Context, exec Executor, id uuid.UUID,
4377
) (models.GraphRelation, error) {
@@ -60,10 +94,11 @@ func (repo *MarbleDbRepository) CreateGraphRelation(ctx context.Context, exec Ex
6094

6195
query := NewQueryBuilder().
6296
Insert(dbmodels.TABLE_GRAPH_RELATIONS).
63-
Columns("id", "org_id", "label", "left_type", "left_field", "right_type", "right_field").
97+
Columns("id", "org_id", "group_id", "label", "left_type", "left_field", "right_type", "right_field").
6498
Values(
6599
pure_utils.NewId(),
66100
relation.OrgId,
101+
relation.GroupId,
67102
relation.Label,
68103
relation.LeftType,
69104
relation.LeftField,

repositories/migrations/20260810081900_graph_walking.sql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
create table graph_relations (
44
id uuid primary key default gen_random_uuid(),
55
org_id uuid not null,
6+
group_id uuid not null,
67
label text not null,
78
left_type text not null,
89
left_field text not null,
@@ -12,7 +13,7 @@ create table graph_relations (
1213

1314
constraint fk_org foreign key (org_id) references organizations (id) on delete cascade,
1415

15-
unique (org_id, label, left_type, left_field, right_type, right_field)
16+
unique (org_id, group_id, left_type, left_field, right_type, right_field)
1617
);
1718

1819
create index idx_graph_relations_org_id on graph_relations (org_id);

usecases/graph_relation_usecase.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/google/uuid"
88

99
"github.com/checkmarble/marble-backend/models"
10+
"github.com/checkmarble/marble-backend/pure_utils"
1011
"github.com/checkmarble/marble-backend/repositories"
1112
"github.com/checkmarble/marble-backend/usecases/executor_factory"
1213
"github.com/checkmarble/marble-backend/usecases/security"
@@ -44,6 +45,24 @@ func (uc GraphRelationUsecase) CreateGraphRelation(ctx context.Context, input mo
4445
return models.GraphRelation{}, err
4546
}
4647

48+
switch input.GroupId {
49+
case uuid.Nil:
50+
input.GroupId = pure_utils.NewId()
51+
52+
default:
53+
// A label belongs to the group, not to the relation, so joining an existing one adopts
54+
// its label rather than setting one. Silently discarding a different label would leave
55+
// the caller believing it took, so say so instead.
56+
label, err := uc.graphRelationRepository.GetGraphRelationGroupLabel(ctx, exec, input.OrgId, input.GroupId)
57+
if err != nil {
58+
return models.GraphRelation{}, err
59+
}
60+
if input.Label != label {
61+
return models.GraphRelation{}, errors.Wrapf(models.BadParameterError,
62+
"group %s is labelled %q, not %q", input.GroupId, label, input.Label)
63+
}
64+
}
65+
4766
endpoints := [][2]string{
4867
{input.LeftType, input.LeftField},
4968
{input.RightType, input.RightField},

0 commit comments

Comments
 (0)