Skip to content

Commit 08d3eb6

Browse files
jhendrixMSFTjhendrixMSFT
authored
Some azappconfig cleanup (Azure#21538)
Improved docs on some APIs. Simplify handling of optional params type. Use context.Background() in tests. Added test for setting with nil value. Fix sort order of fields in options and response types.
1 parent 7117f02 commit 08d3eb6

File tree

5 files changed

+136
-77
lines changed

5 files changed

+136
-77
lines changed

‎sdk/data/azappconfig/assets.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"AssetsRepo": "Azure/azure-sdk-assets",
33
"AssetsRepoPrefixPath": "go",
44
"TagPrefix": "go/data/azappconfig",
5-
"Tag": "go/data/azappconfig_9494a52cb4"
5+
"Tag": "go/data/azappconfig_cc0b6e27f2"
66
}

‎sdk/data/azappconfig/client.go‎

Lines changed: 28 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,16 @@ func (c *Client) UpdateSyncToken(token string) {
8080
}
8181

8282
// AddSetting creates a configuration setting only if the setting does not already exist in the configuration store.
83+
// - ctx controls the lifetime of the HTTP operation
84+
// - key is the name of the setting to create
85+
// - value is the value for the setting. pass nil if the setting doesn't have a value
86+
// - options contains the optional values. can be nil
8387
func (c *Client) AddSetting(ctx context.Context, key string, value *string, options *AddSettingOptions) (AddSettingResponse, error) {
84-
var label *string
85-
var contentType *string
86-
if options != nil {
87-
label = options.Label
88-
contentType = options.ContentType
88+
if options == nil {
89+
options = &AddSettingOptions{}
8990
}
9091

91-
setting := Setting{Key: &key, Value: value, Label: label, ContentType: contentType}
92+
setting := Setting{Key: &key, Value: value, Label: options.Label, ContentType: options.ContentType}
9293

9394
etagAny := azcore.ETagAny
9495
kv, opts := setting.toGeneratedPutOptions(nil, &etagAny)
@@ -105,17 +106,13 @@ func (c *Client) AddSetting(ctx context.Context, key string, value *string, opti
105106

106107
// DeleteSetting deletes a configuration setting from the configuration store.
107108
func (c *Client) DeleteSetting(ctx context.Context, key string, options *DeleteSettingOptions) (DeleteSettingResponse, error) {
108-
var label *string
109-
var ifMatch *azcore.ETag
110-
111-
if options != nil {
112-
label = options.Label
113-
ifMatch = options.OnlyIfUnchanged
109+
if options == nil {
110+
options = &DeleteSettingOptions{}
114111
}
115112

116-
setting := Setting{Key: &key, Label: label}
113+
setting := Setting{Key: &key, Label: options.Label}
117114

118-
resp, err := c.appConfigClient.DeleteKeyValue(ctx, *setting.Key, setting.toGeneratedDeleteOptions(ifMatch))
115+
resp, err := c.appConfigClient.DeleteKeyValue(ctx, *setting.Key, setting.toGeneratedDeleteOptions(options.OnlyIfUnchanged))
119116
if err != nil {
120117
return DeleteSettingResponse{}, err
121118
}
@@ -128,19 +125,13 @@ func (c *Client) DeleteSetting(ctx context.Context, key string, options *DeleteS
128125

129126
// GetSetting retrieves an existing configuration setting from the configuration store.
130127
func (c *Client) GetSetting(ctx context.Context, key string, options *GetSettingOptions) (GetSettingResponse, error) {
131-
var label *string
132-
var ifNoneMatch *azcore.ETag
133-
var acceptDateTime *time.Time
134-
135-
if options != nil {
136-
label = options.Label
137-
ifNoneMatch = options.OnlyIfChanged
138-
acceptDateTime = options.AcceptDateTime
128+
if options == nil {
129+
options = &GetSettingOptions{}
139130
}
140131

141-
setting := Setting{Key: &key, Label: label}
132+
setting := Setting{Key: &key, Label: options.Label}
142133

143-
resp, err := c.appConfigClient.GetKeyValue(ctx, *setting.Key, setting.toGeneratedGetOptions(ifNoneMatch, acceptDateTime))
134+
resp, err := c.appConfigClient.GetKeyValue(ctx, *setting.Key, setting.toGeneratedGetOptions(options.OnlyIfChanged, options.AcceptDateTime))
144135
if err != nil {
145136
return GetSettingResponse{}, err
146137
}
@@ -163,20 +154,16 @@ func (c *Client) GetSetting(ctx context.Context, key string, options *GetSetting
163154

164155
// SetReadOnly sets an existing configuration setting to read only or read write state in the configuration store.
165156
func (c *Client) SetReadOnly(ctx context.Context, key string, isReadOnly bool, options *SetReadOnlyOptions) (SetReadOnlyResponse, error) {
166-
var label *string
167-
var ifMatch *azcore.ETag
168-
169-
if options != nil {
170-
label = options.Label
171-
ifMatch = options.OnlyIfUnchanged
157+
if options == nil {
158+
options = &SetReadOnlyOptions{}
172159
}
173160

174-
setting := Setting{Key: &key, Label: label}
161+
setting := Setting{Key: &key, Label: options.Label}
175162

176163
var err error
177164
if isReadOnly {
178165
var resp generated.AzureAppConfigurationClientPutLockResponse
179-
resp, err = c.appConfigClient.PutLock(ctx, *setting.Key, setting.toGeneratedPutLockOptions(ifMatch))
166+
resp, err = c.appConfigClient.PutLock(ctx, *setting.Key, setting.toGeneratedPutLockOptions(options.OnlyIfUnchanged))
180167
if err == nil {
181168
return SetReadOnlyResponse{
182169
Setting: settingFromGenerated(resp.KeyValue),
@@ -185,7 +172,7 @@ func (c *Client) SetReadOnly(ctx context.Context, key string, isReadOnly bool, o
185172
}
186173
} else {
187174
var resp generated.AzureAppConfigurationClientDeleteLockResponse
188-
resp, err = c.appConfigClient.DeleteLock(ctx, *setting.Key, setting.toGeneratedDeleteLockOptions(ifMatch))
175+
resp, err = c.appConfigClient.DeleteLock(ctx, *setting.Key, setting.toGeneratedDeleteLockOptions(options.OnlyIfUnchanged))
189176
if err == nil {
190177
return SetReadOnlyResponse{
191178
Setting: settingFromGenerated(resp.KeyValue),
@@ -198,20 +185,18 @@ func (c *Client) SetReadOnly(ctx context.Context, key string, isReadOnly bool, o
198185
}
199186

200187
// SetSetting creates a configuration setting if it doesn't exist or overwrites the existing setting in the configuration store.
188+
// - ctx controls the lifetime of the HTTP operation
189+
// - key is the name of the setting to create
190+
// - value is the value for the setting. pass nil if the setting doesn't have a value
191+
// - options contains the optional values. can be nil
201192
func (c *Client) SetSetting(ctx context.Context, key string, value *string, options *SetSettingOptions) (SetSettingResponse, error) {
202-
var label *string
203-
var contentType *string
204-
var ifMatch *azcore.ETag
205-
206-
if options != nil {
207-
label = options.Label
208-
contentType = options.ContentType
209-
ifMatch = options.OnlyIfUnchanged
193+
if options == nil {
194+
options = &SetSettingOptions{}
210195
}
211196

212-
setting := Setting{Key: &key, Value: value, Label: label, ContentType: contentType}
197+
setting := Setting{Key: &key, Value: value, Label: options.Label, ContentType: options.ContentType}
213198

214-
kv, opts := setting.toGeneratedPutOptions(ifMatch, nil)
199+
kv, opts := setting.toGeneratedPutOptions(options.OnlyIfUnchanged, nil)
215200
resp, err := c.appConfigClient.PutKeyValue(ctx, *setting.Key, kv, &opts)
216201
if err != nil {
217202
return SetSettingResponse{}, err

‎sdk/data/azappconfig/client_test.go‎

Lines changed: 95 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ func TestClient(t *testing.T) {
2525
value := "value"
2626
client := NewClientFromConnectionString(t)
2727

28-
addResp, err2 := client.AddSetting(context.TODO(), key, &value, &azappconfig.AddSettingOptions{Label: to.Ptr(label), ContentType: &contentType})
28+
addResp, err2 := client.AddSetting(context.Background(), key, &value, &azappconfig.AddSettingOptions{
29+
Label: to.Ptr(label),
30+
ContentType: &contentType,
31+
})
2932
require.NoError(t, err2)
3033
require.NotEmpty(t, addResp)
3134
require.NotNil(t, addResp.Key)
@@ -37,7 +40,9 @@ func TestClient(t *testing.T) {
3740
require.Equal(t, contentType, *addResp.ContentType)
3841
require.Equal(t, value, *addResp.Value)
3942

40-
getResp, err3 := client.GetSetting(context.TODO(), key, &azappconfig.GetSettingOptions{Label: to.Ptr(label)})
43+
getResp, err3 := client.GetSetting(context.Background(), key, &azappconfig.GetSettingOptions{
44+
Label: to.Ptr(label),
45+
})
4146
require.NoError(t, err3)
4247
require.NotEmpty(t, getResp)
4348
require.NotNil(t, getResp.Key)
@@ -50,13 +55,19 @@ func TestClient(t *testing.T) {
5055
require.Equal(t, value, *getResp.Value)
5156

5257
etag := getResp.ETag
53-
getResp2, err4 := client.GetSetting(context.TODO(), key, &azappconfig.GetSettingOptions{Label: to.Ptr(label), OnlyIfChanged: etag})
58+
getResp2, err4 := client.GetSetting(context.Background(), key, &azappconfig.GetSettingOptions{
59+
Label: to.Ptr(label),
60+
OnlyIfChanged: etag,
61+
})
5462
require.Error(t, err4)
5563
require.Empty(t, getResp2)
5664

5765
value = "value2"
5866
contentType = "content-type2"
59-
setResp, err5 := client.SetSetting(context.TODO(), key, &value, &azappconfig.SetSettingOptions{Label: to.Ptr(label), ContentType: &contentType})
67+
setResp, err5 := client.SetSetting(context.Background(), key, &value, &azappconfig.SetSettingOptions{
68+
Label: to.Ptr(label),
69+
ContentType: &contentType,
70+
})
6071
require.NoError(t, err5)
6172
require.NotEmpty(t, setResp)
6273
require.NotNil(t, setResp.Key)
@@ -72,7 +83,10 @@ func TestClient(t *testing.T) {
7283
// after changing the setting, update the sync token so we don't get cached, stale data
7384
client.UpdateSyncToken(*setResp.SyncToken)
7485

75-
getResp3, err6 := client.GetSetting(context.TODO(), key, &azappconfig.GetSettingOptions{Label: to.Ptr(label), OnlyIfChanged: etag})
86+
getResp3, err6 := client.GetSetting(context.Background(), key, &azappconfig.GetSettingOptions{
87+
Label: to.Ptr(label),
88+
OnlyIfChanged: etag,
89+
})
7690
require.NoError(t, err6)
7791
require.NotEmpty(t, getResp3)
7892
require.NotNil(t, getResp3.Key)
@@ -86,7 +100,10 @@ func TestClient(t *testing.T) {
86100

87101
etag = getResp3.ETag
88102
value = "value3"
89-
setResp2, err7 := client.SetSetting(context.TODO(), key, &value, &azappconfig.SetSettingOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
103+
setResp2, err7 := client.SetSetting(context.Background(), key, &value, &azappconfig.SetSettingOptions{
104+
Label: to.Ptr(label),
105+
OnlyIfUnchanged: etag,
106+
})
90107
require.NoError(t, err7)
91108
require.NotEmpty(t, setResp2)
92109
require.NotNil(t, setResp2.Key)
@@ -101,11 +118,16 @@ func TestClient(t *testing.T) {
101118
// after changing the setting, update the sync token so we don't get cached, stale data
102119
client.UpdateSyncToken(*setResp.SyncToken)
103120

104-
setResp3, err8 := client.SetSetting(context.TODO(), key, &value, &azappconfig.SetSettingOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
121+
setResp3, err8 := client.SetSetting(context.Background(), key, &value, &azappconfig.SetSettingOptions{
122+
Label: to.Ptr(label),
123+
OnlyIfUnchanged: etag,
124+
})
105125
require.Error(t, err8)
106126
require.Empty(t, setResp3)
107127

108-
roResp, err9 := client.SetReadOnly(context.TODO(), key, true, &azappconfig.SetReadOnlyOptions{Label: to.Ptr(label)})
128+
roResp, err9 := client.SetReadOnly(context.Background(), key, true, &azappconfig.SetReadOnlyOptions{
129+
Label: to.Ptr(label),
130+
})
109131
require.NoError(t, err9)
110132
require.NotEmpty(t, roResp)
111133
require.NotNil(t, roResp.Key)
@@ -122,7 +144,9 @@ func TestClient(t *testing.T) {
122144
// after changing the setting, update the sync token so we don't get cached, stale data
123145
client.UpdateSyncToken(*setResp.SyncToken)
124146

125-
roResp2, err10 := client.SetReadOnly(context.TODO(), key, false, &azappconfig.SetReadOnlyOptions{Label: to.Ptr(label)})
147+
roResp2, err10 := client.SetReadOnly(context.Background(), key, false, &azappconfig.SetReadOnlyOptions{
148+
Label: to.Ptr(label),
149+
})
126150
require.NoError(t, err10)
127151
require.NotEmpty(t, roResp2)
128152
require.NotNil(t, roResp2.Key)
@@ -139,12 +163,18 @@ func TestClient(t *testing.T) {
139163
// after changing the setting, update the sync token so we don't get cached, stale data
140164
client.UpdateSyncToken(*setResp.SyncToken)
141165

142-
roResp3, err11 := client.SetReadOnly(context.TODO(), key, true, &azappconfig.SetReadOnlyOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
166+
roResp3, err11 := client.SetReadOnly(context.Background(), key, true, &azappconfig.SetReadOnlyOptions{
167+
Label: to.Ptr(label),
168+
OnlyIfUnchanged: etag,
169+
})
143170
require.Error(t, err11)
144171
require.Empty(t, roResp3)
145172

146173
etag = roResp2.ETag
147-
roResp4, err12 := client.SetReadOnly(context.TODO(), key, true, &azappconfig.SetReadOnlyOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
174+
roResp4, err12 := client.SetReadOnly(context.Background(), key, true, &azappconfig.SetReadOnlyOptions{
175+
Label: to.Ptr(label),
176+
OnlyIfUnchanged: etag,
177+
})
148178
require.NoError(t, err12)
149179
require.NotEmpty(t, roResp4)
150180
require.NotNil(t, roResp4.Key)
@@ -161,12 +191,18 @@ func TestClient(t *testing.T) {
161191
// after changing the setting, update the sync token so we don't get cached, stale data
162192
client.UpdateSyncToken(*setResp.SyncToken)
163193

164-
roResp5, err13 := client.SetReadOnly(context.TODO(), key, false, &azappconfig.SetReadOnlyOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
194+
roResp5, err13 := client.SetReadOnly(context.Background(), key, false, &azappconfig.SetReadOnlyOptions{
195+
Label: to.Ptr(label),
196+
OnlyIfUnchanged: etag,
197+
})
165198
require.Error(t, err13)
166199
require.Empty(t, roResp5)
167200

168201
etag = roResp4.ETag
169-
roResp6, err14 := client.SetReadOnly(context.TODO(), key, false, &azappconfig.SetReadOnlyOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
202+
roResp6, err14 := client.SetReadOnly(context.Background(), key, false, &azappconfig.SetReadOnlyOptions{
203+
Label: to.Ptr(label),
204+
OnlyIfUnchanged: etag,
205+
})
170206
require.NoError(t, err14)
171207
require.NotEmpty(t, roResp6)
172208
require.NotNil(t, roResp6.Key)
@@ -184,29 +220,39 @@ func TestClient(t *testing.T) {
184220
client.UpdateSyncToken(*setResp.SyncToken)
185221

186222
any := "*"
187-
revPgr := client.NewListRevisionsPager(azappconfig.SettingSelector{KeyFilter: &any, LabelFilter: &any, Fields: azappconfig.AllSettingFields()}, nil)
223+
revPgr := client.NewListRevisionsPager(azappconfig.SettingSelector{
224+
KeyFilter: &any,
225+
LabelFilter: &any,
226+
Fields: azappconfig.AllSettingFields(),
227+
}, nil)
188228
require.NotEmpty(t, revPgr)
189229
hasMoreRevs := revPgr.More()
190230
require.True(t, hasMoreRevs)
191-
revResp, err15 := revPgr.NextPage(context.TODO())
231+
revResp, err15 := revPgr.NextPage(context.Background())
192232
require.NoError(t, err15)
193233
require.NotEmpty(t, revResp)
194234
require.Equal(t, key, *revResp.Settings[0].Key)
195235
require.Equal(t, label, *revResp.Settings[0].Label)
196236

197-
settsPgr := client.NewListSettingsPager(azappconfig.SettingSelector{KeyFilter: &any, LabelFilter: &any, Fields: azappconfig.AllSettingFields()}, nil)
237+
settsPgr := client.NewListSettingsPager(azappconfig.SettingSelector{
238+
KeyFilter: &any,
239+
LabelFilter: &any,
240+
Fields: azappconfig.AllSettingFields(),
241+
}, nil)
198242
require.NotEmpty(t, settsPgr)
199243
hasMoreSetts := settsPgr.More()
200244
require.True(t, hasMoreSetts)
201-
settsResp, err16 := settsPgr.NextPage(context.TODO())
245+
settsResp, err16 := settsPgr.NextPage(context.Background())
202246
require.NoError(t, err16)
203247
require.NotEmpty(t, settsResp)
204248
require.Equal(t, key, *settsResp.Settings[0].Key)
205249
require.Equal(t, label, *settsResp.Settings[0].Label)
206250
require.Equal(t, value, *settsResp.Settings[0].Value)
207251
require.False(t, *settsResp.Settings[0].IsReadOnly)
208252

209-
delResp, err17 := client.DeleteSetting(context.TODO(), key, &azappconfig.DeleteSettingOptions{Label: to.Ptr(label)})
253+
delResp, err17 := client.DeleteSetting(context.Background(), key, &azappconfig.DeleteSettingOptions{
254+
Label: to.Ptr(label),
255+
})
210256
require.NoError(t, err17)
211257
require.NotEmpty(t, delResp)
212258
require.NotNil(t, delResp.Key)
@@ -221,7 +267,10 @@ func TestClient(t *testing.T) {
221267
// after changing the setting, update the sync token so we don't get cached, stale data
222268
client.UpdateSyncToken(*setResp.SyncToken)
223269

224-
addResp2, err18 := client.AddSetting(context.TODO(), key, &value, &azappconfig.AddSettingOptions{Label: to.Ptr(label), ContentType: &contentType})
270+
addResp2, err18 := client.AddSetting(context.Background(), key, &value, &azappconfig.AddSettingOptions{
271+
Label: to.Ptr(label),
272+
ContentType: &contentType,
273+
})
225274
require.NoError(t, err18)
226275
require.NotEmpty(t, addResp2)
227276
require.NotNil(t, addResp2.Key)
@@ -233,12 +282,18 @@ func TestClient(t *testing.T) {
233282
require.Equal(t, contentType, *addResp2.ContentType)
234283
require.Equal(t, value, *addResp2.Value)
235284

236-
delResp2, err19 := client.DeleteSetting(context.TODO(), key, &azappconfig.DeleteSettingOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
285+
delResp2, err19 := client.DeleteSetting(context.Background(), key, &azappconfig.DeleteSettingOptions{
286+
Label: to.Ptr(label),
287+
OnlyIfUnchanged: etag,
288+
})
237289
require.Error(t, err19)
238290
require.Empty(t, delResp2)
239291

240292
etag = addResp2.ETag
241-
delResp3, err20 := client.DeleteSetting(context.TODO(), key, &azappconfig.DeleteSettingOptions{Label: to.Ptr(label), OnlyIfUnchanged: etag})
293+
delResp3, err20 := client.DeleteSetting(context.Background(), key, &azappconfig.DeleteSettingOptions{
294+
Label: to.Ptr(label),
295+
OnlyIfUnchanged: etag,
296+
})
242297
require.NoError(t, err20)
243298
require.NotEmpty(t, delResp3)
244299
require.NotNil(t, delResp3.Key)
@@ -250,3 +305,22 @@ func TestClient(t *testing.T) {
250305
require.Equal(t, contentType, *delResp3.ContentType)
251306
require.Equal(t, value, *delResp3.Value)
252307
}
308+
309+
func TestSettingNilValue(t *testing.T) {
310+
const (
311+
key = "key-TestSettingNilValue"
312+
contentType = "content-type"
313+
)
314+
client := NewClientFromConnectionString(t)
315+
316+
addResp, err := client.AddSetting(context.Background(), key, nil, &azappconfig.AddSettingOptions{
317+
ContentType: to.Ptr(contentType),
318+
})
319+
require.NoError(t, err)
320+
require.NotZero(t, addResp)
321+
322+
resp, err := client.DeleteSetting(context.Background(), key, nil)
323+
require.NoError(t, err)
324+
require.NotNil(t, resp.Key)
325+
require.EqualValues(t, key, *resp.Key)
326+
}

0 commit comments

Comments
 (0)