Skip to content

Commit 063b27d

Browse files
authored
fix: handle null getAspect responses, add UpdateColumnDescription (#58)
- getAspect() now returns ErrNotFound when DataHub returns 200 OK with a null or empty aspect value, preventing json.Unmarshal crashes on entities where the aspect has never been written - Add UpdateColumnDescription() for read-modify-write on the editableSchemaMetadata aspect, enabling column-level description edits - Add #nosec G704 annotations on httpClient.Do() calls where URLs are constructed from configured endpoints Closes #57
1 parent 000985d commit 063b27d

File tree

5 files changed

+398
-3
lines changed

5 files changed

+398
-3
lines changed

pkg/client/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (c *Client) doRequest(ctx context.Context, jsonBody []byte, result any) err
175175
req.Header.Set("Content-Type", "application/json")
176176
req.Header.Set("Authorization", "Bearer "+c.token)
177177

178-
resp, err := c.httpClient.Do(req)
178+
resp, err := c.httpClient.Do(req) //#nosec G704 -- URL is constructed from configured endpoint, not arbitrary user input
179179
if err != nil {
180180
return c.handleRequestError(ctx, err)
181181
}

pkg/client/rest.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (c *Client) getAspect(ctx context.Context, entityURN, aspectName string) (j
5858

5959
c.setRESTHeaders(req)
6060

61-
resp, err := c.httpClient.Do(req)
61+
resp, err := c.httpClient.Do(req) //#nosec G704 -- URL is constructed from configured endpoint, not arbitrary user input
6262
if err != nil {
6363
return nil, fmt.Errorf("REST GET failed: %w", err)
6464
}
@@ -84,6 +84,13 @@ func (c *Client) getAspect(ctx context.Context, entityURN, aspectName string) (j
8484
return nil, fmt.Errorf("failed to unmarshal aspect response: %w", err)
8585
}
8686

87+
// DataHub may return 200 OK with a null or empty value when the entity
88+
// exists but the requested aspect has never been written. Treat this the
89+
// same as 404 so callers initialize a default struct.
90+
if isNullOrEmptyJSON(aspectResp.Value) {
91+
return nil, ErrNotFound
92+
}
93+
8794
return aspectResp.Value, nil
8895
}
8996

@@ -125,7 +132,7 @@ func (c *Client) postIngestProposal(ctx context.Context, proposal ingestProposal
125132
c.setRESTHeaders(req)
126133
req.Header.Set("Content-Type", "application/json")
127134

128-
resp, err := c.httpClient.Do(req)
135+
resp, err := c.httpClient.Do(req) //#nosec G704 -- URL is constructed from configured endpoint, not arbitrary user input
129136
if err != nil {
130137
return fmt.Errorf("REST POST failed: %w", err)
131138
}
@@ -151,6 +158,17 @@ func (c *Client) setRESTHeaders(req *http.Request) {
151158
req.Header.Set("X-RestLi-Protocol-Version", "2.0.0")
152159
}
153160

161+
// isNullOrEmptyJSON returns true if the raw JSON message is nil, empty,
162+
// or the JSON literal "null". This covers cases where DataHub returns
163+
// 200 OK but the aspect value is absent.
164+
func isNullOrEmptyJSON(raw json.RawMessage) bool {
165+
if len(raw) == 0 {
166+
return true
167+
}
168+
trimmed := bytes.TrimSpace(raw)
169+
return len(trimmed) == 0 || string(trimmed) == "null"
170+
}
171+
154172
// checkRESTStatus validates REST API response status codes.
155173
func (c *Client) checkRESTStatus(statusCode int, body []byte) error {
156174
switch statusCode {

pkg/client/rest_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,63 @@ func TestGetAspect_NotFound(t *testing.T) {
158158
}
159159
}
160160

161+
func TestGetAspect_NullValue(t *testing.T) {
162+
tests := []struct {
163+
name string
164+
response string
165+
}{
166+
{"null value", `{"value":null}`},
167+
{"empty object no value", `{}`},
168+
}
169+
170+
for _, tt := range tests {
171+
t.Run(tt.name, func(t *testing.T) {
172+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
173+
w.Header().Set("Content-Type", "application/json")
174+
_, _ = w.Write([]byte(tt.response))
175+
}))
176+
defer server.Close()
177+
178+
c := &Client{
179+
endpoint: server.URL + "/api/graphql",
180+
token: "test-token",
181+
httpClient: server.Client(),
182+
logger: NopLogger{},
183+
}
184+
185+
_, err := c.getAspect(context.Background(), "urn:li:dataset:test", "editableDatasetProperties")
186+
if err != ErrNotFound {
187+
t.Errorf("expected ErrNotFound, got: %v", err)
188+
}
189+
})
190+
}
191+
}
192+
193+
func TestIsNullOrEmptyJSON(t *testing.T) {
194+
tests := []struct {
195+
name string
196+
raw json.RawMessage
197+
want bool
198+
}{
199+
{"nil", nil, true},
200+
{"empty bytes", json.RawMessage{}, true},
201+
{"null literal", json.RawMessage(`null`), true},
202+
{"null with spaces", json.RawMessage(` null `), true},
203+
{"valid object", json.RawMessage(`{"key":"val"}`), false},
204+
{"valid array", json.RawMessage(`[]`), false},
205+
{"valid string", json.RawMessage(`"hello"`), false},
206+
}
207+
208+
for _, tt := range tests {
209+
t.Run(tt.name, func(t *testing.T) {
210+
got := isNullOrEmptyJSON(tt.raw)
211+
if got != tt.want {
212+
t.Errorf("isNullOrEmptyJSON(%q) = %v, want %v", string(tt.raw), got, tt.want)
213+
}
214+
})
215+
}
216+
}
217+
161218
func TestGetAspect_Unauthorized(t *testing.T) {
162219
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
163220
w.WriteHeader(http.StatusUnauthorized)

pkg/client/write.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,20 @@ func entityTypeFromURN(urn string) (string, error) {
1717
return parsed.EntityType, nil
1818
}
1919

20+
// editableSchemaAspect is the REST API representation of editableSchemaMetadata.
21+
type editableSchemaAspect struct {
22+
EditableSchemaFieldInfo []editableFieldInfo `json:"editableSchemaFieldInfo"`
23+
}
24+
25+
// editableFieldInfo represents a field's editable metadata for REST API read-modify-write.
26+
// Uses json.RawMessage for tags and glossaryTerms to preserve existing data.
27+
type editableFieldInfo struct {
28+
FieldPath string `json:"fieldPath"`
29+
Description string `json:"description,omitempty"`
30+
GlobalTags json.RawMessage `json:"globalTags,omitempty"`
31+
GlossaryTerms json.RawMessage `json:"glossaryTerms,omitempty"`
32+
}
33+
2034
// editablePropertiesAspect represents the editableDatasetProperties aspect.
2135
type editablePropertiesAspect struct {
2236
Description string `json:"description"`
@@ -336,3 +350,58 @@ func (c *Client) readInstitutionalMemory(ctx context.Context, urn string) (*inst
336350
}
337351
return &memory, nil
338352
}
353+
354+
// UpdateColumnDescription sets the editable description for a specific column
355+
// using read-modify-write on the editableSchemaMetadata aspect.
356+
func (c *Client) UpdateColumnDescription(ctx context.Context, urn, fieldPath, description string) error {
357+
entityType, err := entityTypeFromURN(urn)
358+
if err != nil {
359+
return fmt.Errorf("UpdateColumnDescription: %w", err)
360+
}
361+
362+
schema, err := c.readEditableSchema(ctx, urn)
363+
if err != nil {
364+
return fmt.Errorf("UpdateColumnDescription: %w", err)
365+
}
366+
367+
// Find or create the field entry
368+
found := false
369+
for i := range schema.EditableSchemaFieldInfo {
370+
if schema.EditableSchemaFieldInfo[i].FieldPath == fieldPath {
371+
schema.EditableSchemaFieldInfo[i].Description = description
372+
found = true
373+
break
374+
}
375+
}
376+
if !found {
377+
schema.EditableSchemaFieldInfo = append(schema.EditableSchemaFieldInfo, editableFieldInfo{
378+
FieldPath: fieldPath,
379+
Description: description,
380+
})
381+
}
382+
383+
return c.postIngestProposal(ctx, ingestProposal{
384+
EntityType: entityType,
385+
EntityURN: urn,
386+
AspectName: "editableSchemaMetadata",
387+
Aspect: schema,
388+
})
389+
}
390+
391+
// readEditableSchema reads the current editableSchemaMetadata aspect.
392+
// Returns an empty aspect if none exists (not an error).
393+
func (c *Client) readEditableSchema(ctx context.Context, urn string) (*editableSchemaAspect, error) {
394+
raw, err := c.getAspect(ctx, urn, "editableSchemaMetadata")
395+
if err != nil {
396+
if errors.Is(err, ErrNotFound) {
397+
return &editableSchemaAspect{}, nil
398+
}
399+
return nil, fmt.Errorf("reading editableSchemaMetadata: %w", err)
400+
}
401+
402+
var schema editableSchemaAspect
403+
if err := json.Unmarshal(raw, &schema); err != nil {
404+
return nil, fmt.Errorf("parsing editableSchemaMetadata: %w", err)
405+
}
406+
return &schema, nil
407+
}

0 commit comments

Comments
 (0)