Skip to content

Commit 971abc5

Browse files
enikonenikon
authored
OAS-10653 Inconsistency in Ensure Functions (#662)
* Deprecated EnsureAnalyzer for EnsureCreatedAnalyzer with signature common to all other Ensure* funcitons * Found is not opposite of Create. Both are false when there is an error
1 parent 494e940 commit 971abc5

8 files changed

+106
-28
lines changed

‎Makefile

-7
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,6 @@ endif
8080

8181
TEST_NET := --net=host
8282

83-
# Installation of jq is required for processing AGENCY_DUMP
84-
# ifdef DUMP_AGENCY_ON_FAILURE
85-
# CHECK_JQ_INSTALLTION := $(shell command -v jq >/dev/null 2>&1 || (
86-
#
87-
# endif
88-
89-
9083
# By default we run tests against single endpoint to avoid problems with data propagation in Cluster mode
9184
# e.g. when we create a document in one endpoint, it may not be visible in another endpoint for a while
9285
TEST_ENDPOINTS := http://localhost:7001

‎database_arangosearch_analyzers.go

+5
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,15 @@ type ArangoSearchAnalyzer interface {
4949

5050
type DatabaseArangoSearchAnalyzers interface {
5151

52+
// Deprecated: Use EnsureCreatedAnalyzer instead
5253
// Ensure ensures that the given analyzer exists. If it does not exist it is created.
5354
// The function returns whether the analyzer already existed or an error.
5455
EnsureAnalyzer(ctx context.Context, analyzer ArangoSearchAnalyzerDefinition) (bool, ArangoSearchAnalyzer, error)
5556

57+
// EnsureCreatedAnalyzer creates an Analyzer for the database, if it does not already exist.
58+
// The function returns the Analyser object together with a boolean indicating if the Analyzer was newly created (true) or pre-existing (false).
59+
EnsureCreatedAnalyzer(ctx context.Context, analyzer *ArangoSearchAnalyzerDefinition) (ArangoSearchAnalyzer, bool, error)
60+
5661
// Get returns the analyzer definition for the given analyzer or returns an error
5762
Analyzer(ctx context.Context, name string) (ArangoSearchAnalyzer, error)
5863

‎database_arangosearch_analyzers_impl.go

+36
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ package driver
2424

2525
import (
2626
"context"
27+
"net/http"
2728
"path"
2829
"strings"
2930
)
@@ -90,6 +91,7 @@ func (a *analyzer) Database() Database {
9091
return a.db
9192
}
9293

94+
// Deprecated: Use EnsureCreatedAnalyzer instead
9395
// Ensure ensures that the given analyzer exists. If it does not exist it is created.
9496
// The function returns whether the analyzer already existed or an error.
9597
func (d *database) EnsureAnalyzer(ctx context.Context, definition ArangoSearchAnalyzerDefinition) (bool, ArangoSearchAnalyzer, error) {
@@ -120,6 +122,40 @@ func (d *database) EnsureAnalyzer(ctx context.Context, definition ArangoSearchAn
120122
}, nil
121123
}
122124

125+
// EnsureCreatedAnalyzer creates an Analyzer for the database, if it does not already exist.
126+
// The function returns the Analyser object together with a boolean indicating if the Analyzer was newly created (true) or pre-existing (false).
127+
func (d *database) EnsureCreatedAnalyzer(ctx context.Context, definition *ArangoSearchAnalyzerDefinition) (ArangoSearchAnalyzer, bool, error) {
128+
req, err := d.conn.NewRequest("POST", path.Join(d.relPath(), "_api/analyzer"))
129+
if err != nil {
130+
return nil, false, WithStack(err)
131+
}
132+
applyContextSettings(ctx, req)
133+
req, err = req.SetBody(definition)
134+
if err != nil {
135+
return nil, false, WithStack(err)
136+
}
137+
resp, err := d.conn.Do(ctx, req)
138+
if err != nil {
139+
return nil, false, WithStack(err)
140+
}
141+
142+
if err := resp.CheckStatus(http.StatusCreated, http.StatusOK); err != nil {
143+
return nil, false, WithStack(err)
144+
}
145+
146+
var actualDef ArangoSearchAnalyzerDefinition
147+
if err := resp.ParseBody("", &actualDef); err != nil {
148+
return nil, false, WithStack(err)
149+
}
150+
151+
created := resp.StatusCode() == http.StatusCreated
152+
return &analyzer{
153+
db: d,
154+
definition: actualDef,
155+
}, created, nil
156+
157+
}
158+
123159
// Get returns the analyzer definition for the given analyzer or returns an error
124160
func (d *database) Analyzer(ctx context.Context, name string) (ArangoSearchAnalyzer, error) {
125161
req, err := d.conn.NewRequest("GET", path.Join(d.relPath(), "_api/analyzer/", name))

‎v2/arangodb/collection_indexes_impl.go

+14-14
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ func (c *collectionIndexes) EnsurePersistentIndex(ctx context.Context, fields []
121121
}
122122

123123
result := responseIndex{}
124-
exist, err := c.ensureIndex(ctx, &reqData, &result)
125-
return newIndexResponse(&result), exist, err
124+
created, err := c.ensureIndex(ctx, &reqData, &result)
125+
return newIndexResponse(&result), created, err
126126
}
127127

128128
func (c *collectionIndexes) EnsureGeoIndex(ctx context.Context, fields []string, options *CreateGeoIndexOptions) (IndexResponse, bool, error) {
@@ -137,8 +137,8 @@ func (c *collectionIndexes) EnsureGeoIndex(ctx context.Context, fields []string,
137137
}
138138

139139
result := responseIndex{}
140-
exist, err := c.ensureIndex(ctx, &reqData, &result)
141-
return newIndexResponse(&result), exist, err
140+
created, err := c.ensureIndex(ctx, &reqData, &result)
141+
return newIndexResponse(&result), created, err
142142
}
143143

144144
func (c *collectionIndexes) EnsureTTLIndex(ctx context.Context, fields []string, expireAfter int, options *CreateTTLIndexOptions) (IndexResponse, bool, error) {
@@ -155,8 +155,8 @@ func (c *collectionIndexes) EnsureTTLIndex(ctx context.Context, fields []string,
155155
}
156156

157157
result := responseIndex{}
158-
exist, err := c.ensureIndex(ctx, &reqData, &result)
159-
return newIndexResponse(&result), exist, err
158+
created, err := c.ensureIndex(ctx, &reqData, &result)
159+
return newIndexResponse(&result), created, err
160160
}
161161

162162
func (c *collectionIndexes) EnsureZKDIndex(ctx context.Context, fields []string, options *CreateZKDIndexOptions) (IndexResponse, bool, error) {
@@ -171,8 +171,8 @@ func (c *collectionIndexes) EnsureZKDIndex(ctx context.Context, fields []string,
171171
}
172172

173173
result := responseIndex{}
174-
exist, err := c.ensureIndex(ctx, &reqData, &result)
175-
return newIndexResponse(&result), exist, err
174+
created, err := c.ensureIndex(ctx, &reqData, &result)
175+
return newIndexResponse(&result), created, err
176176
}
177177

178178
func (c *collectionIndexes) EnsureMDIIndex(ctx context.Context, fields []string, options *CreateMDIIndexOptions) (IndexResponse, bool, error) {
@@ -187,8 +187,8 @@ func (c *collectionIndexes) EnsureMDIIndex(ctx context.Context, fields []string,
187187
}
188188

189189
result := responseIndex{}
190-
exist, err := c.ensureIndex(ctx, &reqData, &result)
191-
return newIndexResponse(&result), exist, err
190+
created, err := c.ensureIndex(ctx, &reqData, &result)
191+
return newIndexResponse(&result), created, err
192192
}
193193

194194
func (c *collectionIndexes) EnsureMDIPrefixedIndex(ctx context.Context, fields []string, options *CreateMDIPrefixedIndexOptions) (IndexResponse, bool, error) {
@@ -203,8 +203,8 @@ func (c *collectionIndexes) EnsureMDIPrefixedIndex(ctx context.Context, fields [
203203
}
204204

205205
result := responseIndex{}
206-
exist, err := c.ensureIndex(ctx, &reqData, &result)
207-
return newIndexResponse(&result), exist, err
206+
created, err := c.ensureIndex(ctx, &reqData, &result)
207+
return newIndexResponse(&result), created, err
208208
}
209209

210210
func (c *collectionIndexes) EnsureInvertedIndex(ctx context.Context, options *InvertedIndexOptions) (IndexResponse, bool, error) {
@@ -221,8 +221,8 @@ func (c *collectionIndexes) EnsureInvertedIndex(ctx context.Context, options *In
221221
}
222222

223223
result := responseInvertedIndex{}
224-
exist, err := c.ensureIndex(ctx, &reqData, &result)
225-
return newInvertedIndexResponse(&result), exist, err
224+
created, err := c.ensureIndex(ctx, &reqData, &result)
225+
return newInvertedIndexResponse(&result), created, err
226226
}
227227

228228
func (c *collectionIndexes) ensureIndex(ctx context.Context, reqData interface{}, result interface{}) (bool, error) {

‎v2/arangodb/database_analyzer.go

+5
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,15 @@ import (
2525
)
2626

2727
type DatabaseAnalyzer interface {
28+
// Deprecated: Use EnsureCreatedAnalyzer instead
2829
// EnsureAnalyzer ensures that the given analyzer exists. If it does not exist, it is created.
2930
// The function returns whether the analyzer already existed or not.
3031
EnsureAnalyzer(ctx context.Context, analyzer *AnalyzerDefinition) (bool, Analyzer, error)
3132

33+
// EnsureCreatedAnalyzer creates an Analyzer for the database, if it does not already exist.
34+
// It returns the Analyser object together with a boolean indicating if the Analyzer was newly created (true) or pre-existing (false).
35+
EnsureCreatedAnalyzer(ctx context.Context, analyzer *AnalyzerDefinition) (Analyzer, bool, error)
36+
3237
// Analyzer returns the analyzer definition for the given analyzer
3338
Analyzer(ctx context.Context, name string) (Analyzer, error)
3439

‎v2/arangodb/database_analyzer_impl.go

+23
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ type databaseAnalyzer struct {
4444
db *database
4545
}
4646

47+
// Deprecated: Use EnsureCreatedAnalyzer instead
4748
func (d databaseAnalyzer) EnsureAnalyzer(ctx context.Context, analyzer *AnalyzerDefinition) (bool, Analyzer, error) {
4849
urlEndpoint := d.db.url("_api", "analyzer")
4950

@@ -64,6 +65,28 @@ func (d databaseAnalyzer) EnsureAnalyzer(ctx context.Context, analyzer *Analyzer
6465
}
6566
}
6667

68+
// EnsureCreatedAnalyzer creates an Analyzer for the database, if it does not already exist.
69+
// It returns the Analyser object together with a boolean indicating if the Analyzer was newly created (true) or pre-existing (false).
70+
func (d databaseAnalyzer) EnsureCreatedAnalyzer(ctx context.Context, analyzer *AnalyzerDefinition) (Analyzer, bool, error) {
71+
urlEndpoint := d.db.url("_api", "analyzer")
72+
73+
var response struct {
74+
shared.ResponseStruct `json:",inline"`
75+
AnalyzerDefinition
76+
}
77+
resp, err := connection.CallPost(ctx, d.db.connection(), urlEndpoint, &response, analyzer)
78+
if err != nil {
79+
return nil, false, errors.WithStack(err)
80+
}
81+
82+
switch code := resp.Code(); code {
83+
case http.StatusCreated, http.StatusOK:
84+
return newAnalyzer(d.db, response.AnalyzerDefinition), code == http.StatusCreated, nil
85+
default:
86+
return nil, false, response.AsArangoErrorWithCode(code)
87+
}
88+
}
89+
6790
func (d databaseAnalyzer) Analyzer(ctx context.Context, name string) (Analyzer, error) {
6891
urlEndpoint := d.db.url("_api", "analyzer", url.PathEscape(name))
6992

‎v2/tests/database_analyzer_test.go

+21-5
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func Test_Analyzers(t *testing.T) {
3838
MinVersion *arangodb.Version
3939
Definition arangodb.AnalyzerDefinition
4040
ExpectedDefinition *arangodb.AnalyzerDefinition
41-
Found bool
41+
Created bool
4242
HasError bool
4343
EnterpriseOnly bool
4444
}{
@@ -48,14 +48,15 @@ func Test_Analyzers(t *testing.T) {
4848
Name: "my-identitfy",
4949
Type: arangodb.ArangoSearchAnalyzerTypeIdentity,
5050
},
51+
Created: true,
5152
},
5253
{
5354
Name: "create-again-my-identity",
5455
Definition: arangodb.AnalyzerDefinition{
5556
Name: "my-identitfy",
5657
Type: arangodb.ArangoSearchAnalyzerTypeIdentity,
5758
},
58-
Found: true,
59+
Created: false,
5960
},
6061
{
6162
Name: "create-again-my-identity-diff-type",
@@ -66,6 +67,7 @@ func Test_Analyzers(t *testing.T) {
6667
Delimiter: "äöü",
6768
},
6869
},
70+
Created: false,
6971
HasError: true,
7072
},
7173
{
@@ -78,6 +80,7 @@ func Test_Analyzers(t *testing.T) {
7880
Delimiters: []string{"ö", "ü"},
7981
},
8082
},
83+
Created: true,
8184
HasError: false,
8285
},
8386
{
@@ -89,6 +92,7 @@ func Test_Analyzers(t *testing.T) {
8992
Delimiter: "äöü",
9093
},
9194
},
95+
Created: true,
9296
},
9397
{
9498
Name: "create-my-ngram-3.6",
@@ -116,6 +120,7 @@ func Test_Analyzers(t *testing.T) {
116120
StreamType: utils.NewType(arangodb.ArangoSearchNGramStreamBinary),
117121
},
118122
},
123+
Created: true,
119124
},
120125
{
121126
Name: "create-my-ngram-3.6-custom",
@@ -132,6 +137,7 @@ func Test_Analyzers(t *testing.T) {
132137
StreamType: utils.NewType(arangodb.ArangoSearchNGramStreamUTF8),
133138
},
134139
},
140+
Created: true,
135141
},
136142
{
137143
Name: "create-pipeline-analyzer",
@@ -155,6 +161,7 @@ func Test_Analyzers(t *testing.T) {
155161
},
156162
},
157163
},
164+
Created: true,
158165
},
159166
{
160167
Name: "create-aql-analyzer",
@@ -171,6 +178,7 @@ func Test_Analyzers(t *testing.T) {
171178
MemoryLimit: utils.NewType(1024 * 1024),
172179
},
173180
},
181+
Created: true,
174182
},
175183
{
176184
Name: "create-geopoint",
@@ -188,6 +196,7 @@ func Test_Analyzers(t *testing.T) {
188196
Longitude: []string{},
189197
},
190198
},
199+
Created: true,
191200
},
192201
{
193202
Name: "create-geojson",
@@ -204,6 +213,7 @@ func Test_Analyzers(t *testing.T) {
204213
Type: arangodb.ArangoSearchAnalyzerGeoJSONTypeShape.New(),
205214
},
206215
},
216+
Created: true,
207217
},
208218
{
209219
Name: "create-geo_s2",
@@ -234,6 +244,7 @@ func Test_Analyzers(t *testing.T) {
234244
Type: arangodb.ArangoSearchAnalyzerGeoJSONTypeShape.New(),
235245
},
236246
},
247+
Created: true,
237248
EnterpriseOnly: true,
238249
},
239250
{
@@ -247,6 +258,7 @@ func Test_Analyzers(t *testing.T) {
247258
Case: arangodb.ArangoSearchCaseUpper,
248259
},
249260
},
261+
Created: true,
250262
},
251263
{
252264
Name: "create-collation",
@@ -265,6 +277,7 @@ func Test_Analyzers(t *testing.T) {
265277
Locale: "en_US",
266278
},
267279
},
280+
Created: true,
268281
},
269282
{
270283
Name: "create-stopWords",
@@ -291,6 +304,7 @@ func Test_Analyzers(t *testing.T) {
291304
},
292305
},
293306
},
307+
Created: true,
294308
},
295309
{
296310
Name: "my-minhash",
@@ -330,6 +344,7 @@ func Test_Analyzers(t *testing.T) {
330344
NumHashes: utils.NewType[uint64](2),
331345
},
332346
},
347+
Created: true,
333348
},
334349
{
335350
Name: "create-my-wildcard",
@@ -342,6 +357,7 @@ func Test_Analyzers(t *testing.T) {
342357
},
343358
},
344359
HasError: false,
360+
Created: true,
345361
},
346362
}
347363

@@ -362,15 +378,15 @@ func Test_Analyzers(t *testing.T) {
362378
skipNoEnterprise(client, ctx, t)
363379
}
364380

365-
existed, ensuredA, err := db.EnsureAnalyzer(ctx, &testCase.Definition)
381+
ensuredA, created, err := db.EnsureCreatedAnalyzer(ctx, &testCase.Definition)
366382

367383
if testCase.HasError {
368384
require.Error(t, err)
369385
} else {
370386
require.NoError(t, err)
371387
}
372388

373-
require.Equal(t, testCase.Found, existed)
389+
require.Equal(t, testCase.Created, created)
374390
if ensuredA != nil {
375391
var def arangodb.AnalyzerDefinition
376392
if testCase.ExpectedDefinition != nil {
@@ -417,7 +433,7 @@ func Test_AnalyzerRemove(t *testing.T) {
417433
WithDatabase(t, client, nil, func(db arangodb.Database) {
418434
ctx := context.Background()
419435

420-
_, a, err := db.EnsureAnalyzer(ctx, &def)
436+
a, _, err := db.EnsureCreatedAnalyzer(ctx, &def)
421437
require.NoError(t, err)
422438

423439
// delete and check it was deleted (use force to delete it even if it is in use)

0 commit comments

Comments
 (0)