FerretDB · AlekSi · Jul 20, 2023 · Jul 20, 2023
@@ -210,6 +210,7 @@ tasks:
         go test -count=1 -run='{{or .TEST_RUN .SHARD_RUN}}' -timeout={{.TEST_TIMEOUT}} {{.RACE_FLAG}} -tags={{.BUILD_TAGS}} -shuffle=on -coverpkg=../...
         -coverprofile=integration-sqlite.txt .
         -target-backend=ferretdb-sqlite
+        -sqlite-url=file:../tmp/sqlite-tests/
         -target-tls
         -compat-url='mongodb://username:password@127.0.0.1:47018/?tls=true&tlsCertificateKeyFile=../build/certs/client.pem&tlsCaFile=../build/certs/rootCA-cert.pem'
         -disable-filter-pushdown

@@ -27,6 +27,7 @@ tasks:
         go test -count=1 {{.RACE_FLAG}} -run=TestEnvData
         -tags=ferretdb_testenvdata .
         -target-backend=ferretdb-sqlite
+        -sqlite-url=file:../tmp/sqlite-tests/
       # no hana yet
       - >
         go test -count=1 {{.RACE_FLAG}} -run=TestEnvData
@@ -86,6 +87,7 @@ tasks:
         -log-level=error
         -bench-docs={{.BENCH_DOCS}}
         -target-backend=ferretdb-sqlite
+        -sqlite-url=file:../tmp/sqlite-tests/
         | tee new-sqlite.txt
       - ../bin/benchstat{{exeExt}} old-sqlite.txt new-sqlite.txt
 
@@ -98,6 +100,7 @@ tasks:
         -log-level=error
         -bench-docs={{.BENCH_DOCS}}
         -target-backend=ferretdb-sqlite
+        -sqlite-url=file:../tmp/sqlite-tests/
         -- -disable-filter-pushdown
         | tee new-sqlite.txt
       - ../bin/benchstat{{exeExt}} old-sqlite.txt new-sqlite.txt

@@ -107,16 +107,19 @@ func setupListener(tb testtb.TB, ctx context.Context, logger *zap.Logger) string
 	switch *targetBackendF {
 	case "ferretdb-pg":
 		require.NotEmpty(tb, *postgreSQLURLF, "-postgresql-url must be set for %q", *targetBackendF)
+		require.Empty(tb, *sqliteURLF, "-sqlite-url must be empty for %q", *targetBackendF)
 		require.Empty(tb, *hanaURLF, "-hana-url must be empty for %q", *targetBackendF)
 		handler = "pg"
 
 	case "ferretdb-sqlite":
 		require.Empty(tb, *postgreSQLURLF, "-postgresql-url must be empty for %q", *targetBackendF)
+		require.NotEmpty(tb, *sqliteURLF, "-sqlite-url must be set for %q", *targetBackendF)
 		require.Empty(tb, *hanaURLF, "-hana-url must be empty for %q", *targetBackendF)
 		handler = "sqlite"
 
 	case "ferretdb-hana":
 		require.Empty(tb, *postgreSQLURLF, "-postgresql-url must be empty for %q", *targetBackendF)
+		require.Empty(tb, *sqliteURLF, "-sqlite-url must be empty for %q", *targetBackendF)
 		require.NotEmpty(tb, *hanaURLF, "-hana-url must be set for %q", *targetBackendF)
 		handler = "hana"
 
@@ -137,10 +140,8 @@ func setupListener(tb testtb.TB, ctx context.Context, logger *zap.Logger) string
 		StateProvider: p,
 
 		PostgreSQLURL: *postgreSQLURLF,
-
-		SQLiteURL: sqliteURL.String(),
-
-		HANAURL: *hanaURLF,
+		SQLiteURL:     *sqliteURLF,
+		HANAURL:       *hanaURLF,
 
 		TestOpts: registry.TestOpts{
 			DisableFilterPushdown: *disableFilterPushdownF,

@@ -48,6 +48,7 @@ var (
 	targetUnixSocketF = flag.Bool("target-unix-socket", false, "in-process FerretDB: use Unix socket")
 
 	postgreSQLURLF = flag.String("postgresql-url", "", "in-process FerretDB: PostgreSQL URL for 'pg' handler.")
+	sqliteURLF     = flag.String("sqlite-url", "", "in-process FerretDB: SQLite URI for 'sqlite' handler.")
 	hanaURLF       = flag.String("hana-url", "", "in-process FerretDB: Hana URL for 'hana' handler.")
 
 	compatURLF = flag.String("compat-url", "", "compat system's (MongoDB) URL for compatibility tests; if empty, they are skipped")

@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"path/filepath"
 	"runtime"
 	"time"
 
@@ -44,11 +45,6 @@ var listenerMetrics = connmetrics.NewListenerMetrics()
 // jaegerExporter is a shared Jaeger exporter for tests.
 var jaegerExporter *jaeger.Exporter
 
-// sqliteURL is a URI for SQLite backend tests.
-//
-// We don't use testing.T.TempDir() or something to make debugging of failed tests easier.
-var sqliteURL = must.NotFail(url.Parse("file:../tmp/sqlite-tests/"))
-
 // Startup initializes things that should be initialized only once.
 func Startup() {
 	logging.Setup(zap.DebugLevel, "")
@@ -83,10 +79,20 @@ func Startup() {
 		zap.S().Fatalf("Unknown target backend %q.", *targetBackendF)
 	}
 
-	must.BeTrue(sqliteURL.Path == "")
-	must.BeTrue(sqliteURL.Opaque != "")
-	_ = os.Remove(sqliteURL.Opaque)
-	must.NoError(os.MkdirAll(sqliteURL.Opaque, 0o777))
+	if *sqliteURLF != "" {
+		u, err := url.Parse(*sqliteURLF)
+		if err != nil {
+			zap.S().Fatalf("Failed to parse -sqlite-url %s: %s", u, err)
+		}
+
+		must.BeTrue(u.Path == "")
+		must.BeTrue(u.Opaque != "")
+
+		dir := must.NotFail(filepath.Abs(u.Opaque))
+		zap.S().Infof("Recreating %s", dir)
+		_ = os.Remove(dir)
+		must.NoError(os.MkdirAll(dir, 0o777))
+	}
 
 	if u := *targetURLF; u != "" {
 		client, err := makeClient(ctx, u)

@@ -29,7 +29,12 @@ type db struct {
 	token *resource.Token
 }
 
-// openDB opens existing database.
+// openDB opens existing database or creates a new one.
+//
+// All valid FerretDB database names are valid SQLite database names / file names,
+// so no validation is needed.
+// One exception is very long full path names for the filesystem,
+// but we don't check it.
 func openDB(uri string) (*db, error) {
 	sqlDB, err := sql.Open("sqlite", uri)
 	if err != nil {

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package pool provides access to SQLite database connections.
+// Package pool provides access to SQLite databases and their connections.
 //
 // It should be used only by the metadata package.
 package pool
@@ -39,7 +39,7 @@ import (
 // filenameExtension represents SQLite database filename extension.
 const filenameExtension = ".sqlite"
 
-// Pool provides access to SQLite database connections.
+// Pool provides access to SQLite databases and their connections.
 //
 //nolint:vet // for readability
 type Pool struct {
@@ -52,9 +52,11 @@ type Pool struct {
 	token *resource.Token
 }
 
-// New creates a pool for SQLite databases in the given directory.
+// New creates a pool for SQLite databases in the directory specified by SQLite URI.
+//
+// All databases are opened on creation.
 func New(u string, l *zap.Logger) (*Pool, error) {
-	uri, err := validateURI(u)
+	uri, err := parseURI(u)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse SQLite URI %q: %s", u, err)
 	}
@@ -91,50 +93,6 @@ func New(u string, l *zap.Logger) (*Pool, error) {
 	return p, nil
 }
 
-// validateURI checks given URI value and returns parsed URL.
-// URI should contain 'file' scheme and point to an existing directory.
-// Path should end with '/'. Authority should be empty or absent.
-//
-// Returned URL contains path in both Path and Opaque to make String() method work correctly.
-func validateURI(u string) (*url.URL, error) {
-	uri, err := url.Parse(u)
-	if err != nil {
-		return nil, err
-	}
-
-	if uri.Scheme != "file" {
-		return nil, fmt.Errorf(`expected "file:" schema, got %q`, uri.Scheme)
-	}
-
-	if uri.User != nil {
-		return nil, fmt.Errorf(`expected empty user info, got %q`, uri.User)
-	}
-
-	if uri.Host != "" {
-		return nil, fmt.Errorf(`expected empty host, got %q`, uri.Host)
-	}
-
-	if uri.Path == "" && uri.Opaque != "" {
-		uri.Path = uri.Opaque
-	}
-	uri.Opaque = uri.Path
-
-	if !strings.HasSuffix(uri.Path, "/") {
-		return nil, fmt.Errorf(`expected path ending with "/", got %q`, uri.Path)
-	}
-
-	fi, err := os.Stat(uri.Path)
-	if err != nil {
-		return nil, fmt.Errorf(`%q should be an existing directory, got %s`, uri.Path, err)
-	}
-
-	if !fi.IsDir() {
-		return nil, fmt.Errorf(`%q should be an existing directory`, uri.Path)
-	}
-
-	return uri, nil
-}
-
 // databaseName returns database name for given database file path.
 func (p *Pool) databaseName(databaseFile string) string {
 	return strings.TrimSuffix(filepath.Base(databaseFile), filenameExtension)
@@ -179,7 +137,7 @@ func (p *Pool) List(ctx context.Context) []string {
 	return res
 }
 
-// GetExisting returns an existing database connection by name, or nil.
+// GetExisting returns an existing database by valid name, or nil.
 func (p *Pool) GetExisting(ctx context.Context, name string) *sql.DB {
 	p.rw.RLock()
 	defer p.rw.RUnlock()
@@ -192,9 +150,9 @@ func (p *Pool) GetExisting(ctx context.Context, name string) *sql.DB {
 	return db.sqlDB
 }
 
-// GetOrCreate returns an existing database connection by name, or creates a new one.
+// GetOrCreate returns an existing database by valid name, or creates a new one.
 //
-// Returned boolean value indicates whether the connection was created.
+// Returned boolean value indicates whether the database was created.
 func (p *Pool) GetOrCreate(ctx context.Context, name string) (*sql.DB, bool, error) {
 	sqlDB := p.GetExisting(ctx, name)
 	if sqlDB != nil {
@@ -222,7 +180,7 @@ func (p *Pool) GetOrCreate(ctx context.Context, name string) (*sql.DB, bool, err
 	return db.sqlDB, true, nil
 }
 
-// Drop closes and removes a database by name.
+// Drop closes and removes a database by valid name.
 //
 // It does nothing if the database does not exist.
 //

@@ -15,11 +15,8 @@
 package pool
 
 import (
-	"net/url"
-	"os"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/FerretDB/FerretDB/internal/util/testutil"
@@ -28,7 +25,8 @@ import (
 func TestCreateDrop(t *testing.T) {
 	ctx := testutil.Ctx(t)
 
-	p, err := New("file:"+t.TempDir()+"/", testutil.Logger(t))
+	// that also tests that query parameters are preserved
+	p, err := New("file:/?mode=memory", testutil.Logger(t))
 	require.NoError(t, err)
 
 	defer p.Close()
@@ -58,127 +56,3 @@ func TestCreateDrop(t *testing.T) {
 	db = p.GetExisting(ctx, t.Name())
 	require.Nil(t, db)
 }
-
-func TestNewBackend(t *testing.T) {
-	t.Parallel()
-
-	err := os.MkdirAll("tmp/dir", 0o777)
-	require.NoError(t, err)
-
-	_, err = os.Create("tmp/file")
-	require.NoError(t, err)
-
-	t.Cleanup(func() {
-		err := os.RemoveAll("tmp")
-		require.NoError(t, err)
-	})
-
-	testCases := map[string]struct {
-		value string
-		uri   *url.URL
-		err   string
-	}{
-		"LocalDirectory": {
-			value: "file:./",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "./",
-				Path:   "./",
-			},
-		},
-		"LocalSubDirectory": {
-			value: "file:./tmp/",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "./tmp/",
-				Path:   "./tmp/",
-			},
-		},
-		"LocalSubSubDirectory": {
-			value: "file:./tmp/dir/",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "./tmp/dir/",
-				Path:   "./tmp/dir/",
-			},
-		},
-		"LocalDirectoryWithParameters": {
-			value: "file:./tmp/?mode=ro",
-			uri: &url.URL{
-				Scheme:   "file",
-				Opaque:   "./tmp/",
-				Path:     "./tmp/",
-				RawQuery: "mode=ro",
-			},
-		},
-		"AbsoluteDirectory": {
-			value: "file:/tmp/",
-			uri: &url.URL{
-				Scheme:   "file",
-				Opaque:   "/tmp/",
-				Path:     "/tmp/",
-				OmitHost: true,
-			},
-		},
-		"WithEmptyAuthority": {
-			value: "file:///tmp/",
-			uri: &url.URL{
-				Scheme: "file",
-				Opaque: "/tmp/",
-				Path:   "/tmp/",
-			},
-		},
-		"WithEmptyAuthorityAndQuery": {
-			value: "file:///tmp/?mode=ro",
-			uri: &url.URL{
-				Scheme:   "file",
-				Opaque:   "/tmp/",
-				Path:     "/tmp/",
-				RawQuery: "mode=ro",
-			},
-		},
-		"HostIsNotEmpty": {
-			value: "file://localhost/./tmp/?mode=ro",
-			err:   `expected empty host, got "localhost"`,
-		},
-		"UserIsNotEmpty": {
-			value: "file://user:pass@./tmp/?mode=ro",
-			err:   `expected empty user info, got "user:pass"`,
-		},
-		"NoDirectory": {
-			value: "file:./nodir/",
-			err:   `"./nodir/" should be an existing directory, got stat ./nodir/: no such file or directory`,
-		},
-		"PathIsNotEndsWithSlash": {
-			value: "file:./tmp/file",
-			err:   `expected path ending with "/", got "./tmp/file"`,
-		},
-		"FileInsteadOfDirectory": {
-			value: "file:./tmp/file/",
-			err:   `"./tmp/file/" should be an existing directory, got stat ./tmp/file/: not a directory`,
-		},
-		"MalformedURI": {
-			value: ":./tmp/",
-			err:   `parse ":./tmp/": missing protocol scheme`,
-		},
-		"NoScheme": {
-			value: "./tmp/",
-			err:   `expected "file:" schema, got ""`,
-		},
-	}
-	for name, tc := range testCases {
-		name, tc := name, tc
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			u, err := validateURI(tc.value)
-			if tc.err != "" {
-				assert.EqualError(t, err, tc.err)
-				return
-			}
-
-			require.NoError(t, err)
-			assert.Equal(t, u, tc.uri)
-		})
-	}
-}