Skip to content

Commit f501f32

Browse files
srenatussrenatus
committed
database: only include adjacent "subfolders" in results
This might be a good compromise between extra effort and usability. Signed-off-by: Stephan Renatus <[email protected]>
1 parent 03dab88 commit f501f32

File tree

5 files changed

+220
-195
lines changed

5 files changed

+220
-195
lines changed

‎internal/database/database.go‎

Lines changed: 61 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"fmt"
1313
"io/fs"
1414
"iter"
15-
"log"
1615
"maps"
1716
"os"
1817
"path/filepath"
@@ -335,7 +334,7 @@ func (d *Database) CloseDB() {
335334

336335
func (d *Database) SourcesDataGet(ctx context.Context, sourceName, path string, principal string) (any, []string, error) {
337336
path = filepath.ToSlash(path)
338-
return tx3(ctx, d, sourcesDataGet(ctx, d, sourceName, path, principal,
337+
return tx3(ctx, d, sourcesDataGet(ctx, d, true, sourceName, path, principal,
339338
func(bs []byte) (data any, err error) {
340339
return data, json.Unmarshal(bs, &data)
341340
}))
@@ -344,7 +343,7 @@ func (d *Database) SourcesDataGet(ctx context.Context, sourceName, path string,
344343
func (d *Database) SourcesDataPatch(ctx context.Context, sourceName, path string, principal string, patch jsonpatch.Patch) error {
345344
path = filepath.ToSlash(path)
346345
return tx1(ctx, d, func(tx *sql.Tx) error {
347-
previous, _, err := sourcesDataGet(ctx, d, sourceName, path, principal, func(bs []byte) ([]byte, error) { return bs, nil })(tx)
346+
previous, _, err := sourcesDataGet(ctx, d, false, sourceName, path, principal, func(bs []byte) ([]byte, error) { return bs, nil })(tx)
348347
if err != nil {
349348
return err
350349
}
@@ -356,7 +355,7 @@ func (d *Database) SourcesDataPatch(ctx context.Context, sourceName, path string
356355
})
357356
}
358357

359-
func sourcesDataGet[T any](ctx context.Context, d *Database, sourceName, path string, principal string,
358+
func sourcesDataGet[T any](ctx context.Context, d *Database, returnFiles bool, sourceName, path string, principal string,
360359
f func([]byte) (T, error),
361360
) func(*sql.Tx) (T, []string, error) {
362361
return func(tx *sql.Tx) (T, []string, error) {
@@ -375,57 +374,58 @@ func sourcesDataGet[T any](ctx context.Context, d *Database, sourceName, path st
375374
return zero, nil, err
376375
}
377376

378-
conditions, args := expr.SQL(d.arg, []any{sourceName, path})
379-
380-
offset := len(args)
381-
upwardsPaths := upwardsPaths(path)
382-
if len(upwardsPaths) == 0 {
383-
return zero, nil, err
384-
}
385-
inParams := make([]string, len(upwardsPaths))
386-
for i := range upwardsPaths {
387-
inParams[i] = d.arg(i + 1 + offset)
388-
}
389-
377+
var files []string
390378
prefix := filepath.Dir(path)
391379

392-
values := make([]any, 0, len(args)+3)
393-
values = append(values, sourceName, "x")
394-
values = append(values, args[2:]...) // conditions
395-
values = append(values, prefix+"/%")
396-
values = append(values, upwardsPaths...)
380+
conditions, args := expr.SQL(d.arg, []any{sourceName, path})
381+
if returnFiles { // get "directory" contents
382+
offset := len(prefix) + 2
383+
query := fmt.Sprintf(`SELECT DISTINCT
384+
SUBSTRING(
385+
SUBSTRING(path, %[1]d),
386+
1,
387+
CASE
388+
WHEN %[2]s > 0
389+
THEN %[2]s - 1
390+
ELSE LENGTH(SUBSTRING(path, %[1]d))
391+
END
392+
) AS entry
393+
FROM sources_data
394+
WHERE source_name = %[3]s
395+
AND path <> %[4]s
396+
AND (%[5]s)
397+
AND path LIKE %[6]s
398+
LIMIT 25`, // limit 25 is arbitrary: there could be a LOT of entries...
399+
offset, d.locate("'/'", fmt.Sprintf("SUBSTRING(path, %d)", offset)),
400+
d.arg(0), d.arg(1), conditions, d.arg(len(args)))
401+
402+
files, err = queryPaths(ctx, tx, query, append(args, prefix+"/%")...)
403+
if err != nil {
404+
if !errors.Is(err, sql.ErrNoRows) { // no rows, no nested files to return
405+
return zero, nil, err
406+
}
407+
}
408+
}
397409

398-
query := fmt.Sprintf(`SELECT path FROM sources_data
410+
// get file content at node
411+
args[1] = path
412+
query := fmt.Sprintf(`SELECT data FROM sources_data
399413
WHERE source_name = %s
400-
AND (%s <> '')
414+
AND path = %s
401415
AND (%s)
402-
AND (path LIKE %s OR path in (%s))
403416
ORDER BY path`,
404-
d.arg(0), d.arg(1), conditions, d.arg(offset), strings.Join(inParams, ","))
417+
d.arg(0), d.arg(1), conditions)
405418

406-
log.Println(query, values)
407-
408-
files, err := queryPaths(ctx, tx, query, values...)
409-
if err != nil {
410-
if !errors.Is(err, sql.ErrNoRows) { // no rows, no problem
411-
return zero, nil, err
419+
var bs []byte
420+
if err := tx.QueryRowContext(ctx, query, args...).Scan(&bs); err != nil {
421+
if errors.Is(err, sql.ErrNoRows) { // no rows, no response data
422+
return zero, files, nil
412423
}
413-
}
414-
log.Printf("%d files", len(files))
415-
416-
if len(files) == 0 {
417-
return zero, nil, err
418-
}
419-
420-
fs0 := sourcedatafs.New(ctx, files, func(file string) func(context.Context) ([]byte, error) {
421-
return d.sourceData(tx, sourceName, file)
422-
})
423-
result, err := loader.NewFileLoader().WithFS(fs0).All([]string{"."})
424-
if err != nil {
425424
return zero, files, err
426425
}
427-
var a any = result.Documents
428-
return a.(T), files, nil
426+
427+
ret, err := f(bs)
428+
return ret, files, err
429429
}
430430
}
431431

@@ -533,6 +533,9 @@ ORDER BY path LIMIT 4`,
533533

534534
files, err := queryPaths(ctx, tx, query, values...)
535535
if err != nil {
536+
if errors.Is(err, sql.ErrNoRows) { // no rows, no conflict
537+
return nil
538+
}
536539
return err
537540
}
538541
if len(files) == 0 {
@@ -610,7 +613,8 @@ func (d *Database) SourcesDataDelete(ctx context.Context, sourceName, path strin
610613

611614
conditions, args := expr.SQL(d.arg, []any{sourceName, path})
612615

613-
_, err = tx.Exec(fmt.Sprintf(`DELETE FROM sources_data WHERE source_name = %s AND path = %s AND (`+conditions+")", d.arg(0), d.arg(1)), args...)
616+
query := fmt.Sprintf(`DELETE FROM sources_data WHERE source_name = %s AND path = %s AND (%s)`, d.arg(0), d.arg(1), conditions)
617+
_, err = tx.ExecContext(ctx, query, args...)
614618
return err
615619
})
616620
}
@@ -781,7 +785,7 @@ func (d *Database) ListBundles(ctx context.Context, principal string, opts ListO
781785
args = append(args, opts.Limit)
782786
}
783787

784-
rows, err := txn.Query(query, args...)
788+
rows, err := txn.QueryContext(ctx, query, args...)
785789
if err != nil {
786790
return nil, "", err
787791
}
@@ -1839,6 +1843,16 @@ func (d *Database) arg(i int) string {
18391843
return "?"
18401844
}
18411845

1846+
func (d *Database) locate(needle string, haystack string) string {
1847+
switch d.kind {
1848+
case sqlite:
1849+
return "INSTR(" + haystack + ", " + needle + ")"
1850+
case postgres:
1851+
return "POSITION(" + needle + " IN " + haystack + ")"
1852+
}
1853+
return "LOCATE(" + needle + ", " + haystack + ")"
1854+
}
1855+
18421856
func (d *Database) args(n int) []string {
18431857
args := make([]string, n)
18441858
for i := range n {

‎internal/database/database_test.go‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package database_test
22

33
import (
44
"context"
5+
"path"
56
"reflect"
7+
"slices"
68
"testing"
79

810
"github.com/google/go-cmp/cmp"
@@ -266,15 +268,19 @@ func TestDatabase(t *testing.T) {
266268
GetStack("stack1", nil),
267269

268270
// source data operations:
269-
newTestCase("source/get non-existing data").SourcesGetData("system1", "foo", nil),
270-
newTestCase("source/put source data").SourcesPutData("system1", "foo", data1).SourcesGetData("system1", "foo", data1),
271-
newTestCase("source/update data foo").SourcesPutData("system1", "foo", data2).SourcesGetData("system1", "foo", data2),
272-
newTestCase("source/update data bar").SourcesPutData("system1", "bar", data1).SourcesGetData("system1", "bar", data1),
271+
newTestCase("source/get non-existing data").SourcesGetData("system1", "foo", nil, nil),
272+
newTestCase("source/put source data").SourcesPutData("system1", "foo", data1).SourcesGetData("system1", "foo", data1, nil),
273+
newTestCase("source/update data foo").SourcesPutData("system1", "foo", data2).SourcesGetData("system1", "foo", data2, nil),
274+
newTestCase("source/update data bar").
275+
SourcesPutData("system1", "subtree/a/b", data1).
276+
SourcesGetData("system1", "subtree", nil, []string{"a"}).
277+
SourcesGetData("system1", "subtree/a", nil, []string{"b"}).
278+
SourcesGetData("system1", "subtree/a/b", data1, nil),
273279
newTestCase("source/query data").SourcesQueryData("system1", map[string][]byte{
274-
"bar": []byte(`{"key":"value1"}`),
275-
"foo": []byte(`{"key":"value2"}`),
280+
"foo/data.json": []byte(`{"key":"value2"}`),
281+
"subtree/a/b/data.json": []byte(`{"key":"value1"}`),
276282
}),
277-
newTestCase("source/delete data").SourcesDeleteData("system1", "foo").SourcesGetData("system1", "foo", nil),
283+
newTestCase("source/delete data").SourcesDeleteData("system1", "foo").SourcesGetData("system1", "foo", nil, nil),
278284
newTestCase("source/put requirements").SourcesPutRequirements("system1", config.Requirements{
279285
config.Requirement{Source: newString("system2")},
280286
config.Requirement{Source: newString("system3")},
@@ -400,25 +406,30 @@ func (tc *testCase) GetPrincipalByToken(token, exp string) *testCase {
400406
return tc
401407
}
402408

403-
func (tc *testCase) SourcesGetData(srcID, dataID string, expected any) *testCase {
409+
func (tc *testCase) SourcesGetData(srcID, dataID string, expectedData any, expectedChildren []string) *testCase {
404410
tc.operations = append(tc.operations, func(ctx context.Context, t *testing.T, db *database.Database) {
405-
data, found, err := db.SourcesDataGet(ctx, srcID, dataID, "admin")
411+
data, adjacent, err := db.SourcesDataGet(ctx, srcID, path.Join(dataID, "data.json"), "admin")
406412
if err != nil {
407413
t.Fatalf("expected no error, got %v", err)
408414
}
415+
found := data != nil
409416

410417
switch {
411-
case found && expected == nil:
418+
case found && expectedData == nil:
412419
t.Fatal("expected no data to be found")
413-
case !found && expected != nil:
414-
t.Fatal("expected data to be found")
415-
case !found && expected == nil:
420+
case !found && expectedData != nil:
421+
t.Fatal("expectedData data to be found")
422+
case !found && expectedData == nil:
416423
// OK
417-
case found && expected != nil:
418-
if !reflect.DeepEqual(expected, data) {
424+
case found && expectedData != nil:
425+
if !reflect.DeepEqual(expectedData, data) {
419426
t.Fatalf("expected data not found, got %v", data)
420427
}
421428
}
429+
430+
if !slices.Equal(expectedChildren, adjacent) {
431+
t.Fatalf("expected children %v, got %v", expectedChildren, expectedData)
432+
}
422433
})
423434
return tc
424435
}
@@ -434,15 +445,15 @@ func (tc *testCase) SourcesQueryData(srcID string, expected map[string][]byte) *
434445
}
435446

436447
if !reflect.DeepEqual(expected, data) {
437-
t.Fatalf("expected data not found, got %v", data)
448+
t.Fatalf("expected data %v, got %v", expected, data)
438449
}
439450
})
440451
return tc
441452
}
442453

443454
func (tc *testCase) SourcesPutData(srcID, dataID string, data any) *testCase {
444455
tc.operations = append(tc.operations, func(ctx context.Context, t *testing.T, db *database.Database) {
445-
if err := db.SourcesDataPut(ctx, srcID, dataID, data, "admin"); err != nil {
456+
if err := db.SourcesDataPut(ctx, srcID, path.Join(dataID, "data.json"), data, "admin"); err != nil {
446457
t.Fatalf("expected no error, got %v", err)
447458
}
448459
})
@@ -451,7 +462,7 @@ func (tc *testCase) SourcesPutData(srcID, dataID string, data any) *testCase {
451462

452463
func (tc *testCase) SourcesDeleteData(srcID, dataID string) *testCase {
453464
tc.operations = append(tc.operations, func(ctx context.Context, t *testing.T, db *database.Database) {
454-
if err := db.SourcesDataDelete(ctx, srcID, dataID, "admin"); err != nil {
465+
if err := db.SourcesDataDelete(ctx, srcID, path.Join(dataID, "data.json"), "admin"); err != nil {
455466
t.Fatalf("expected no error, got %v", err)
456467
}
457468
})

‎internal/server/server.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -294,14 +294,10 @@ func (s *Server) v1SourcesDataGet(w http.ResponseWriter, r *http.Request) {
294294
return
295295
}
296296

297-
resp := types.SourcesGetDataResponseV1{}
298-
299-
if len(paths) > 0 {
297+
resp := types.SourcesGetDataResponseV1{Paths: paths}
298+
if data != nil {
300299
resp.Result = &data
301300
}
302-
if len(paths)> 1 {
303-
resp.Paths = paths
304-
}
305301

306302
JSONOK(w, resp, pretty(r))
307303
}

0 commit comments

Comments
 (0)