Skip to content

Commit d199566

Browse files
authored
Fix UpdateDescription to use read-modify-write pattern (#46)
* fix: handle WriteRune return values in toEnumCase Satisfy revive unhandled-error linter. * fix: UpdateDescription uses read-modify-write to preserve audit stamps DataHub v1.3.0 rejects partial editableDatasetProperties aspects that are missing created/lastModified audit stamps. Read the existing aspect first, update the description field, and write back the complete structure — matching the pattern used by all other write operations. Closes #45 * test: validate UpdateDescription preserves audit stamps and handles markdown Add tests for read-modify-write pattern: existing properties with audit stamps preserved, no existing properties (404 fallback), markdown content with bold/italic/lists/code, and invalid JSON error handling.
1 parent 2aa1a75 commit d199566

File tree

3 files changed

+148
-12
lines changed

3 files changed

+148
-12
lines changed

pkg/client/options.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ func toEnumCase(s string) string {
2727
for i, r := range runes {
2828
// Insert underscore before uppercase if preceded by lowercase
2929
if i > 0 && unicode.IsUpper(r) && unicode.IsLower(runes[i-1]) {
30-
result.WriteRune('_')
30+
_, _ = result.WriteRune('_')
3131
}
32-
result.WriteRune(unicode.ToUpper(r))
32+
_, _ = result.WriteRune(unicode.ToUpper(r))
3333
}
3434
return result.String()
3535
}

pkg/client/write.go

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

20-
// UpdateDescription sets the editable description for any entity.
20+
// editablePropertiesAspect represents the editableDatasetProperties aspect.
21+
type editablePropertiesAspect struct {
22+
Description string `json:"description"`
23+
Created *auditStampRaw `json:"created,omitempty"`
24+
LastModified *auditStampRaw `json:"lastModified,omitempty"`
25+
}
26+
27+
// UpdateDescription sets the editable description for any entity using read-modify-write.
2128
func (c *Client) UpdateDescription(ctx context.Context, urn, description string) error {
2229
entityType, err := entityTypeFromURN(urn)
2330
if err != nil {
2431
return fmt.Errorf("UpdateDescription: %w", err)
2532
}
2633

27-
aspect := map[string]any{
28-
"description": description,
34+
props, err := c.readEditableProperties(ctx, urn)
35+
if err != nil {
36+
return fmt.Errorf("UpdateDescription: %w", err)
2937
}
3038

39+
props.Description = description
40+
3141
return c.postIngestProposal(ctx, ingestProposal{
3242
EntityType: entityType,
3343
EntityURN: urn,
3444
AspectName: "editableDatasetProperties",
35-
Aspect: aspect,
45+
Aspect: props,
3646
})
3747
}
3848

49+
// readEditableProperties reads the current editableDatasetProperties aspect.
50+
// Returns an empty aspect if none exists (not an error).
51+
func (c *Client) readEditableProperties(ctx context.Context, urn string) (*editablePropertiesAspect, error) {
52+
raw, err := c.getAspect(ctx, urn, "editableDatasetProperties")
53+
if err != nil {
54+
if errors.Is(err, ErrNotFound) {
55+
return &editablePropertiesAspect{}, nil
56+
}
57+
return nil, fmt.Errorf("reading editableDatasetProperties: %w", err)
58+
}
59+
60+
var props editablePropertiesAspect
61+
if err := json.Unmarshal(raw, &props); err != nil {
62+
return nil, fmt.Errorf("parsing editableDatasetProperties: %w", err)
63+
}
64+
return &props, nil
65+
}
66+
3967
// globalTagsAspect represents the globalTags aspect structure.
4068
type globalTagsAspect struct {
4169
Tags []tagAssociation `json:"tags"`

pkg/client/write_test.go

Lines changed: 114 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,14 @@ import (
1010

1111
func TestUpdateDescription(t *testing.T) {
1212
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13-
if r.Method != http.MethodPost {
14-
t.Errorf("expected POST, got %s", r.Method)
13+
if r.Method == http.MethodGet {
14+
propsJSON := `{"description":"old desc",` +
15+
`"created":{"actor":"urn:li:corpuser:admin","time":1000},` +
16+
`"lastModified":{"actor":"urn:li:corpuser:admin","time":2000}}`
17+
resp := aspectResponse{Value: json.RawMessage(propsJSON)}
18+
w.Header().Set("Content-Type", "application/json")
19+
_ = json.NewEncoder(w).Encode(resp)
20+
return
1521
}
1622

1723
proposal, aspectJSON := extractProposalWireFormat(t, r.Body)
@@ -23,12 +29,18 @@ func TestUpdateDescription(t *testing.T) {
2329
t.Errorf("expected aspect 'editableDatasetProperties', got %v", proposal["aspectName"])
2430
}
2531

26-
var aspectMap map[string]any
27-
if err := json.Unmarshal([]byte(aspectJSON), &aspectMap); err != nil {
32+
var props editablePropertiesAspect
33+
if err := json.Unmarshal([]byte(aspectJSON), &props); err != nil {
2834
t.Fatalf("failed to unmarshal inner aspect: %v", err)
2935
}
30-
if aspectMap["description"] != "new description" {
31-
t.Errorf("expected description 'new description', got %v", aspectMap["description"])
36+
if props.Description != "new description" {
37+
t.Errorf("expected 'new description', got %q", props.Description)
38+
}
39+
if props.Created == nil || props.Created.Actor != "urn:li:corpuser:admin" {
40+
t.Error("expected created audit stamp to be preserved")
41+
}
42+
if props.LastModified == nil || props.LastModified.Time != 2000 {
43+
t.Error("expected lastModified audit stamp to be preserved")
3244
}
3345

3446
w.WriteHeader(http.StatusOK)
@@ -50,6 +62,102 @@ func TestUpdateDescription(t *testing.T) {
5062
}
5163
}
5264

65+
func TestUpdateDescription_NoExistingProperties(t *testing.T) {
66+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
67+
if r.Method == http.MethodGet {
68+
w.WriteHeader(http.StatusNotFound)
69+
return
70+
}
71+
_, aspectJSON := extractProposalWireFormat(t, r.Body)
72+
var props editablePropertiesAspect
73+
if err := json.Unmarshal([]byte(aspectJSON), &props); err != nil {
74+
t.Fatalf("failed to unmarshal inner aspect: %v", err)
75+
}
76+
if props.Description != "first description" {
77+
t.Errorf("expected 'first description', got %q", props.Description)
78+
}
79+
w.WriteHeader(http.StatusOK)
80+
}))
81+
defer server.Close()
82+
83+
c := &Client{
84+
endpoint: server.URL + "/api/graphql",
85+
token: "test-token",
86+
httpClient: server.Client(),
87+
logger: NopLogger{},
88+
}
89+
90+
err := c.UpdateDescription(context.Background(),
91+
"urn:li:dataset:(urn:li:dataPlatform:hive,testdb.table,PROD)",
92+
"first description")
93+
if err != nil {
94+
t.Fatalf("unexpected error: %v", err)
95+
}
96+
}
97+
98+
func TestUpdateDescription_MarkdownContent(t *testing.T) {
99+
mdDesc := "## Overview\n\n**bold** and _italic_\n\n- item 1\n- item 2\n\n`code`"
100+
101+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
102+
if r.Method == http.MethodGet {
103+
propsJSON := `{"description":"old",` +
104+
`"created":{"actor":"urn:li:corpuser:datahub","time":0},` +
105+
`"lastModified":{"actor":"urn:li:corpuser:datahub","time":0}}`
106+
resp := aspectResponse{Value: json.RawMessage(propsJSON)}
107+
w.Header().Set("Content-Type", "application/json")
108+
_ = json.NewEncoder(w).Encode(resp)
109+
return
110+
}
111+
_, aspectJSON := extractProposalWireFormat(t, r.Body)
112+
var props editablePropertiesAspect
113+
if err := json.Unmarshal([]byte(aspectJSON), &props); err != nil {
114+
t.Fatalf("failed to unmarshal inner aspect: %v", err)
115+
}
116+
if props.Description != mdDesc {
117+
t.Errorf("markdown not preserved:\ngot: %q\nwant: %q",
118+
props.Description, mdDesc)
119+
}
120+
w.WriteHeader(http.StatusOK)
121+
}))
122+
defer server.Close()
123+
124+
c := &Client{
125+
endpoint: server.URL + "/api/graphql",
126+
token: "test-token",
127+
httpClient: server.Client(),
128+
logger: NopLogger{},
129+
}
130+
131+
err := c.UpdateDescription(context.Background(),
132+
"urn:li:dataset:(urn:li:dataPlatform:hive,testdb.table,PROD)",
133+
mdDesc)
134+
if err != nil {
135+
t.Fatalf("unexpected error: %v", err)
136+
}
137+
}
138+
139+
func TestReadEditableProperties_InvalidJSON(t *testing.T) {
140+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
141+
resp := aspectResponse{Value: json.RawMessage(`not valid json`)}
142+
w.Header().Set("Content-Type", "application/json")
143+
_ = json.NewEncoder(w).Encode(resp)
144+
}))
145+
defer server.Close()
146+
147+
c := &Client{
148+
endpoint: server.URL + "/api/graphql",
149+
token: "test-token",
150+
httpClient: server.Client(),
151+
logger: NopLogger{},
152+
}
153+
154+
_, err := c.readEditableProperties(context.Background(),
155+
"urn:li:dataset:(urn:li:dataPlatform:hive,testdb.table,PROD)")
156+
if err == nil {
157+
t.Fatal("expected error for invalid JSON")
158+
}
159+
}
160+
53161
func TestUpdateDescription_InvalidURN(t *testing.T) {
54162
c := &Client{logger: NopLogger{}}
55163
err := c.UpdateDescription(context.Background(), "not-a-urn", "desc")

0 commit comments

Comments
 (0)