Tidying up

go/libraries/doltcore/sqle/database_provider.go

@@ -662,6 +662,7 @@ func (p *DoltDatabaseProvider) registerNewDatabase(ctx *sql.Context, name string
 	// mutex is locked (without actually locking it and causing a deadlock) and to error out if we detect
 	// that the mutex is NOT locked.
 	if p.mu.TryLock() {
+		defer p.mu.Unlock()
 		return fmt.Errorf("unable to register new database without database provider mutex being locked")
 	}
 

go/libraries/doltcore/sqle/dropped_databases.go

@@ -175,7 +175,6 @@ func (dd *droppedDatabaseManager) ListDroppedDatabases(_ *sql.Context) ([]string
 	databaseNames := make([]string, 0, 5)
 	callback := func(path string, size int64, isDir bool) (stop bool) {
 		_, lastPathSegment := filepath.Split(path)
-		// TODO: Is there a common util we use for this somewhere?
 		lastPathSegment = strings.ReplaceAll(lastPathSegment, "-", "_")
 		databaseNames = append(databaseNames, lastPathSegment)
 		return false

go/libraries/doltcore/sqle/dsess/session_db_provider.go

@@ -102,7 +102,6 @@ type DoltDatabaseProvider interface {
 	// (e.g. a root database will be restored as a regular/non-root database,
 	// databases original stored with hyphens in their directory name will be rewritten
 	// to underscores to match their SQL database name).
-	// TODO: Is this a problem for anything on hosted?
 	// If the database is unable to be restored, an error is returned explaining why.
 	UndropDatabase(ctx *sql.Context, dbName string) error
 	// ListDroppedDatabases returns a list of the database names for dropped databases that are still

go/libraries/utils/filesys/fs_test.go

@@ -128,12 +128,12 @@ func TestFilesystems(t *testing.T) {
 			err = fs.MoveDir(subdir, filepath.Join(newLocation, "subdir"))
 			require.NoError(t, err)
 
-			// Assert that nothing exists as the old path
+			// Assert that nothing exists at the old path
 			exists, isDir = fs.Exists(subdir)
 			require.False(t, exists)
 			require.False(t, isDir)
 
-			// Assert that our directory exists as the new path
+			// Assert that our directory exists at the new path
 			exists, isDir = fs.Exists(filepath.Join(newLocation, "subdir"))
 			require.True(t, exists)
 			require.True(t, isDir)

go/libraries/utils/filesys/inmemfs.go

@@ -518,6 +518,13 @@ func (fs *InMemFS) MoveDir(srcPath, destPath string) error {
 }
 
 func (fs *InMemFS) moveDirHelper(dir *memDir, destPath string) error {
+	// All calls to moveDirHelper should happen with the filesystem's read-write mutex locked;
+	// if we detect the mutex isn't locked, then return an error.
+	if fs.rwLock.TryLock() {
+		defer fs.rwLock.Unlock()
+		return fmt.Errorf("moveDirHelper called without first aquiring filesystem read-write lock")
+	}
+
 	if _, exists := fs.objs[destPath]; exists {
 		return fmt.Errorf("destination path exists: %s", destPath)
 	}
@@ -589,6 +596,13 @@ func (fs *InMemFS) MoveFile(srcPath, destPath string) error {
 }
 
 func (fs *InMemFS) moveFileHelper(obj *memFile, destPath string) error {
+	// All calls to moveFileHelper should happen with the filesystem's read-write mutex locked;
+	// if we detect the mutex isn't locked, then return an error.
+	if fs.rwLock.TryLock() {
+		defer fs.rwLock.Unlock()
+		return fmt.Errorf("moveFileHelper called without first aquiring filesystem read-write lock")
+	}
+
 	destDir := filepath.Dir(destPath)
 	destParentDir, err := fs.mkDirs(destDir)
 	if err != nil {

go/libraries/utils/filesys/localfs.go

@@ -288,10 +288,6 @@ func (fs *localFS) MoveFile(srcPath, destPath string) (err error) {
 }
 
 func (fs *localFS) MoveDir(srcPath, destPath string) (err error) {
-	// TODO: This is the exact same implementation as MoveFile
-	//       Should probably at least add assertions that |srcPath| is really a dir?
-	// TODO: Or should we just try to make MoveFile work with dirs? It seems like the filesystem
-	//       based implementation already does, it's just the in-memory implementation that doesn't.
 	srcPath, err = fs.Abs(srcPath)
 	if err != nil {
 		return err

integration-tests/bats/undrop.bats

@@ -40,6 +40,14 @@ teardown() {
   [ $status -eq 1 ]
   [[ $output =~ "dolt_undrop called with too many arguments" ]] || false
   [[ $output =~ "dolt_undrop only accepts one argument - the name of the dropped database to restore" ]] || false
+
+  # Create and drop a database to test error messages when there is a db available to undrop
+  dolt sql -q "create database dropper;"
+  dolt sql -q "drop database dropper;"
+  run dolt sql -q "CALL dolt_undrop();"
+  [ $status -eq 1 ]
+  [[ $output =~ "no database name specified." ]] || false
+  [[ $output =~ " available databases that can be undropped: dropper" ]] || false
 }
 
 @test "undrop: purge error messages" {