Skip to content

Commit 994577a

Browse files
authored
fix: resolve three DataHub 1.4.x write method bugs (#106)
* fix: resolve three DataHub 1.4.x write method bugs - Add GraphQL selection set to upsertStructuredProperties and removeStructuredProperties mutations (fixes SubselectionRequired error) - Change raiseIncident to use singular resourceUrn field instead of resourceUrns array (fixes "at least 1 resource urn" error) - Expand entity-type support in write methods to include domain, glossaryTerm, and glossaryNode for tags, glossary terms, and descriptions (fixes "unsupported entity type" errors) * fix: use "definition" field for glossaryTermInfo description aspect GlossaryTermInfo PDL schema uses "definition" not "description" for the term text, matching the existing glossaryNodeInfo mapping.
1 parent 06816e9 commit 994577a

File tree

7 files changed

+131
-48
lines changed

7 files changed

+131
-48
lines changed

pkg/client/incidents.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,14 @@ func (c *Client) GetIncidents(ctx context.Context, urn string) (*types.IncidentR
105105

106106
// RaiseIncident creates a new incident on entities.
107107
func (c *Client) RaiseIncident(ctx context.Context, input types.RaiseIncidentInput) (string, error) {
108-
resourceURNs := make([]map[string]string, 0, len(input.ResourceURNs))
109-
for _, urn := range input.ResourceURNs {
110-
resourceURNs = append(resourceURNs, map[string]string{"urn": urn})
108+
if len(input.ResourceURNs) == 0 {
109+
return "", fmt.Errorf("RaiseIncident: at least one resource URN is required")
111110
}
112111

113112
gqlInput := map[string]any{
114-
"type": input.Type,
115-
"title": input.Title,
116-
"resourceUrns": resourceURNs,
113+
"type": input.Type,
114+
"title": input.Title,
115+
"resourceUrn": input.ResourceURNs[0],
117116
}
118117
if input.CustomType != "" {
119118
gqlInput["customType"] = input.CustomType

pkg/client/incidents_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,10 +286,35 @@ func TestRaiseIncident(t *testing.T) {
286286
if !tt.wantErr && urn != tt.wantURN {
287287
t.Errorf("URN = %q, want %q", urn, tt.wantURN)
288288
}
289+
290+
// Verify resourceUrn is sent as a singular string field
291+
if !tt.wantErr && receivedBody != nil {
292+
vars, _ := receivedBody["variables"].(map[string]any)
293+
input, _ := vars["input"].(map[string]any)
294+
resourceUrn, ok := input["resourceUrn"].(string)
295+
if !ok {
296+
t.Fatal("expected resourceUrn to be a string")
297+
}
298+
if resourceUrn != tt.input.ResourceURNs[0] {
299+
t.Errorf("resourceUrn = %q, want %q", resourceUrn, tt.input.ResourceURNs[0])
300+
}
301+
}
289302
})
290303
}
291304
}
292305

306+
func TestRaiseIncident_EmptyResourceURNs(t *testing.T) {
307+
c := &Client{logger: NopLogger{}}
308+
_, err := c.RaiseIncident(context.Background(), types.RaiseIncidentInput{
309+
Type: "OPERATIONAL",
310+
Title: "Test",
311+
ResourceURNs: []string{},
312+
})
313+
if err == nil {
314+
t.Fatal("expected error for empty resource URNs")
315+
}
316+
}
317+
293318
func TestResolveIncident(t *testing.T) {
294319
tests := []struct {
295320
name string

pkg/client/structured_properties.go

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,16 +126,30 @@ query listStructuredPropertyDefinitions($input: SearchInput!) {
126126
`
127127

128128
// UpsertStructuredPropertiesMutation sets or updates structured property values on an entity.
129+
// The mutation returns StructuredProperties! which requires a selection set.
129130
UpsertStructuredPropertiesMutation = `
130131
mutation upsertStructuredProperties($input: UpsertStructuredPropertiesInput!) {
131-
upsertStructuredProperties(input: $input)
132+
upsertStructuredProperties(input: $input) {
133+
properties {
134+
structuredProperty {
135+
urn
136+
}
137+
}
138+
}
132139
}
133140
`
134141

135142
// RemoveStructuredPropertiesMutation removes structured property values from an entity.
143+
// The mutation returns StructuredProperties! which requires a selection set.
136144
RemoveStructuredPropertiesMutation = `
137145
mutation removeStructuredProperties($input: RemoveStructuredPropertiesInput!) {
138-
removeStructuredProperties(input: $input)
146+
removeStructuredProperties(input: $input) {
147+
properties {
148+
structuredProperty {
149+
urn
150+
}
151+
}
152+
}
139153
}
140154
`
141155
)
@@ -232,7 +246,13 @@ func (c *Client) UpsertStructuredProperties(ctx context.Context, urn string, pro
232246
}
233247

234248
var response struct {
235-
UpsertStructuredProperties bool `json:"upsertStructuredProperties"`
249+
UpsertStructuredProperties struct {
250+
Properties []struct {
251+
StructuredProperty struct {
252+
URN string `json:"urn"`
253+
} `json:"structuredProperty"`
254+
} `json:"properties"`
255+
} `json:"upsertStructuredProperties"`
236256
}
237257

238258
if err := c.Execute(ctx, UpsertStructuredPropertiesMutation, variables, &response); err != nil {
@@ -252,7 +272,13 @@ func (c *Client) RemoveStructuredProperties(ctx context.Context, urn string, pro
252272
}
253273

254274
var response struct {
255-
RemoveStructuredProperties bool `json:"removeStructuredProperties"`
275+
RemoveStructuredProperties struct {
276+
Properties []struct {
277+
StructuredProperty struct {
278+
URN string `json:"urn"`
279+
} `json:"structuredProperty"`
280+
} `json:"properties"`
281+
} `json:"removeStructuredProperties"`
256282
}
257283

258284
if err := c.Execute(ctx, RemoveStructuredPropertiesMutation, variables, &response); err != nil {

pkg/client/structured_properties_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ func TestUpsertStructuredProperties(t *testing.T) {
500500
Values: []any{float64(30)},
501501
},
502502
},
503-
response: `{"data": {"upsertStructuredProperties": true}}`,
503+
response: `{"data":{"upsertStructuredProperties":{"properties":[{"structuredProperty":{"urn":"urn:li:structuredProperty:t"}}]}}}`,
504504
},
505505
{
506506
name: "multiple properties",
@@ -515,7 +515,7 @@ func TestUpsertStructuredProperties(t *testing.T) {
515515
Values: []any{"PII", "SENSITIVE"},
516516
},
517517
},
518-
response: `{"data": {"upsertStructuredProperties": true}}`,
518+
response: `{"data":{"upsertStructuredProperties":{"properties":[{"structuredProperty":{"urn":"urn:li:structuredProperty:t"}}]}}}`,
519519
},
520520
{
521521
name: "graphql error",
@@ -580,7 +580,7 @@ func TestRemoveStructuredProperties(t *testing.T) {
580580
name: "single property",
581581
urn: "urn:li:dataset:test",
582582
propertyURNs: []string{"urn:li:structuredProperty:retentionTime"},
583-
response: `{"data": {"removeStructuredProperties": true}}`,
583+
response: `{"data": {"removeStructuredProperties": {"properties": []}}}`,
584584
},
585585
{
586586
name: "multiple properties",
@@ -589,7 +589,7 @@ func TestRemoveStructuredProperties(t *testing.T) {
589589
"urn:li:structuredProperty:retentionTime",
590590
"urn:li:structuredProperty:classification",
591591
},
592-
response: `{"data": {"removeStructuredProperties": true}}`,
592+
response: `{"data": {"removeStructuredProperties": {"properties": []}}}`,
593593
},
594594
{
595595
name: "graphql error",

pkg/client/write.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ type DescriptionAspectInfo struct {
4848
// descriptionAspectMap maps DataHub entity types to their description aspect.
4949
// glossaryNode uses "definition" instead of "description".
5050
// dataProduct uses its non-editable property aspect (dataProductProperties).
51-
// domain and glossaryTerm are intentionally excluded: their aspects
52-
// (domainProperties, glossaryTermInfo) are not writable via the REST ingest proposal API.
51+
// domain uses domainProperties and glossaryTerm uses glossaryTermInfo.
5352
var descriptionAspectMap = map[string]DescriptionAspectInfo{
5453
entityTypeDataset: {AspectName: "editableDatasetProperties", FieldName: "description"},
5554
entityTypeDashboard: {AspectName: "editableDashboardProperties", FieldName: "description"},
@@ -59,6 +58,8 @@ var descriptionAspectMap = map[string]DescriptionAspectInfo{
5958
entityTypeContainer: {AspectName: "editableContainerProperties", FieldName: "description"},
6059
entityTypeDataProduct: {AspectName: "dataProductProperties", FieldName: "description"},
6160
entityTypeGlossaryNode: {AspectName: "glossaryNodeInfo", FieldName: "definition"},
61+
entityTypeDomain: {AspectName: "domainProperties", FieldName: "description"},
62+
entityTypeGlossaryTerm: {AspectName: "glossaryTermInfo", FieldName: "definition"},
6263
}
6364

6465
// globalTagsSupportedTypes lists entity types that support the globalTags aspect.
@@ -67,6 +68,7 @@ var descriptionAspectMap = map[string]DescriptionAspectInfo{
6768
var globalTagsSupportedTypes = map[string]bool{
6869
entityTypeDataset: true, entityTypeDashboard: true, entityTypeChart: true,
6970
entityTypeDataFlow: true, entityTypeDataJob: true, entityTypeContainer: true, entityTypeDataProduct: true,
71+
entityTypeDomain: true, entityTypeGlossaryTerm: true, entityTypeGlossaryNode: true,
7072
}
7173

7274
// glossaryTermsSupportedTypes lists entity types that support glossaryTerms associations.
@@ -75,6 +77,7 @@ var globalTagsSupportedTypes = map[string]bool{
7577
var glossaryTermsSupportedTypes = map[string]bool{
7678
entityTypeDataset: true, entityTypeDashboard: true, entityTypeChart: true,
7779
entityTypeDataFlow: true, entityTypeDataJob: true, entityTypeContainer: true, entityTypeDataProduct: true,
80+
entityTypeDomain: true, entityTypeGlossaryTerm: true, entityTypeGlossaryNode: true,
7881
}
7982

8083
// institutionalMemorySupportedTypes lists entity types that support institutional memory (links).

pkg/client/write_test.go

Lines changed: 61 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,43 @@ func TestAddTag_NoExistingTags(t *testing.T) {
304304
}
305305
}
306306

307+
func TestAddTag_DomainEntity(t *testing.T) {
308+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
309+
if r.Method == http.MethodGet {
310+
w.WriteHeader(http.StatusNotFound)
311+
return
312+
}
313+
proposal, aspectJSON := extractProposalWireFormat(t, r.Body)
314+
if proposal["entityType"] != "domain" {
315+
t.Errorf("entityType = %v, want domain", proposal["entityType"])
316+
}
317+
if proposal["aspectName"] != "globalTags" {
318+
t.Errorf("aspectName = %v, want globalTags", proposal["aspectName"])
319+
}
320+
var tags globalTagsAspect
321+
if err := json.Unmarshal([]byte(aspectJSON), &tags); err != nil {
322+
t.Fatalf("failed to unmarshal inner aspect: %v", err)
323+
}
324+
if len(tags.Tags) != 1 {
325+
t.Errorf("expected 1 tag, got %d", len(tags.Tags))
326+
}
327+
w.WriteHeader(http.StatusOK)
328+
}))
329+
defer server.Close()
330+
331+
c := &Client{
332+
endpoint: server.URL + "/api/graphql",
333+
token: "test-token",
334+
httpClient: server.Client(),
335+
logger: NopLogger{},
336+
}
337+
338+
err := c.AddTag(context.Background(), "urn:li:domain:engineering", "urn:li:tag:PII")
339+
if err != nil {
340+
t.Fatalf("unexpected error: %v", err)
341+
}
342+
}
343+
307344
func TestRemoveTag(t *testing.T) {
308345
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
309346
if r.Method == http.MethodGet {
@@ -1234,9 +1271,9 @@ func TestLookupDescriptionAspect(t *testing.T) {
12341271
{"container", "editableContainerProperties", "description", false},
12351272
{"dataProduct", "dataProductProperties", "description", false},
12361273
{"glossaryNode", "glossaryNodeInfo", "definition", false},
1274+
{"domain", "domainProperties", "description", false},
1275+
{"glossaryTerm", "glossaryTermInfo", "definition", false},
12371276
// Unsupported types return errors
1238-
{"domain", "", "", true},
1239-
{"glossaryTerm", "", "", true},
12401277
{"tag", "", "", true},
12411278
{"corpuser", "", "", true},
12421279
}
@@ -1287,6 +1324,20 @@ func TestUpdateDescription_EntityTypes(t *testing.T) {
12871324
wantAspect: "editableContainerProperties",
12881325
wantField: "description",
12891326
},
1327+
{
1328+
name: "domain",
1329+
urn: "urn:li:domain:engineering",
1330+
wantEntity: "domain",
1331+
wantAspect: "domainProperties",
1332+
wantField: "description",
1333+
},
1334+
{
1335+
name: "glossaryTerm",
1336+
urn: "urn:li:glossaryTerm:Classification",
1337+
wantEntity: "glossaryTerm",
1338+
wantAspect: "glossaryTermInfo",
1339+
wantField: "definition",
1340+
},
12901341
}
12911342

12921343
for _, tt := range tests {
@@ -1356,36 +1407,14 @@ func TestUpdateDescription_UnsupportedEntityType(t *testing.T) {
13561407
}
13571408
}
13581409

1359-
func TestUpdateDescription_DomainReturnsError(t *testing.T) {
1360-
c := &Client{logger: NopLogger{}}
1361-
err := c.UpdateDescription(context.Background(), "urn:li:domain:engineering", "desc")
1362-
if err == nil {
1363-
t.Fatal("expected error for domain entity")
1364-
}
1365-
if !errors.Is(err, ErrUnsupportedEntityType) {
1366-
t.Errorf("expected ErrUnsupportedEntityType, got: %v", err)
1367-
}
1368-
}
1369-
1370-
func TestUpdateDescription_GlossaryTermReturnsError(t *testing.T) {
1371-
c := &Client{logger: NopLogger{}}
1372-
err := c.UpdateDescription(context.Background(), "urn:li:glossaryTerm:Classification", "desc")
1373-
if err == nil {
1374-
t.Fatal("expected error for glossaryTerm entity")
1375-
}
1376-
if !errors.Is(err, ErrUnsupportedEntityType) {
1377-
t.Errorf("expected ErrUnsupportedEntityType, got: %v", err)
1378-
}
1379-
}
1380-
13811410
func TestAddTag_UnsupportedEntityType(t *testing.T) {
13821411
c := &Client{logger: NopLogger{}}
13831412
tests := []struct {
13841413
name string
13851414
urn string
13861415
}{
1387-
{"domain", "urn:li:domain:engineering"},
1388-
{"glossaryTerm", "urn:li:glossaryTerm:Classification"},
1416+
{"tag", "urn:li:tag:PII"},
1417+
{"corpuser", "urn:li:corpuser:johndoe"},
13891418
}
13901419
for _, tt := range tests {
13911420
t.Run(tt.name, func(t *testing.T) {
@@ -1406,8 +1435,8 @@ func TestRemoveTag_UnsupportedEntityType(t *testing.T) {
14061435
name string
14071436
urn string
14081437
}{
1409-
{"domain", "urn:li:domain:engineering"},
1410-
{"glossaryTerm", "urn:li:glossaryTerm:Classification"},
1438+
{"tag", "urn:li:tag:PII"},
1439+
{"corpuser", "urn:li:corpuser:johndoe"},
14111440
}
14121441
for _, tt := range tests {
14131442
t.Run(tt.name, func(t *testing.T) {
@@ -1428,8 +1457,8 @@ func TestAddGlossaryTerm_UnsupportedEntityType(t *testing.T) {
14281457
name string
14291458
urn string
14301459
}{
1431-
{"domain", "urn:li:domain:engineering"},
1432-
{"glossaryTerm", "urn:li:glossaryTerm:Classification"},
1460+
{"tag", "urn:li:tag:PII"},
1461+
{"corpuser", "urn:li:corpuser:johndoe"},
14331462
}
14341463
for _, tt := range tests {
14351464
t.Run(tt.name, func(t *testing.T) {
@@ -1450,8 +1479,8 @@ func TestRemoveGlossaryTerm_UnsupportedEntityType(t *testing.T) {
14501479
name string
14511480
urn string
14521481
}{
1453-
{"domain", "urn:li:domain:engineering"},
1454-
{"glossaryTerm", "urn:li:glossaryTerm:Classification"},
1482+
{"tag", "urn:li:tag:PII"},
1483+
{"corpuser", "urn:li:corpuser:johndoe"},
14551484
}
14561485
for _, tt := range tests {
14571486
t.Run(tt.name, func(t *testing.T) {

pkg/types/incidents.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,6 @@ type RaiseIncidentInput struct {
6060
Description string `json:"description,omitempty"`
6161

6262
// ResourceURNs are the entity URNs affected by this incident.
63+
// Only the first element is sent to the DataHub API (resourceUrn field).
6364
ResourceURNs []string `json:"resource_urns"`
6465
}

0 commit comments

Comments
 (0)