Refactor file imports, fixes #666, fixes #865 (#1037)

cmd/ddev/cmd/import-files.go

@@ -1,51 +1,108 @@
 package cmd
 
 import (
-	"os"
+	"fmt"
 
+	"github.com/drud/ddev/pkg/appimport"
 	"github.com/drud/ddev/pkg/ddevapp"
 	"github.com/drud/ddev/pkg/dockerutil"
+	"github.com/drud/ddev/pkg/output"
 	"github.com/drud/ddev/pkg/util"
 	"github.com/spf13/cobra"
 )
 
-var fileSource string
-var fileExtPath string
+var sourcePath string
+var extPath string
 
 // ImportFileCmd represents the `ddev import-db` command.
 var ImportFileCmd = &cobra.Command{
 	Use:   "import-files",
 	Short: "Import the uploaded files directory of an existing project to the default public upload directory of your project.",
-	Long: `Import the uploaded files directory of an existing project to the default public
-upload directory of your project. The files can be provided as a directory
-path or an archive in .tar, .tar.gz, .tgz, or .zip format. For the .zip and tar formats,
-the path to a directory within the archive can be provided if it is not located at the
-top-level of the archive. If the destination directory exists, it will be replaced with
-the assets being imported.`,
+	Long: `Import the uploaded files directory of an existing project to the default
+public upload directory of your project. The files can be provided as a
+directory path or an archive in .tar, .tar.gz, .tgz, or .zip format. For the
+.zip and tar formats, the path to a directory within the archive can be
+provided if it is not located at the top-level of the archive. If the
+destination directory exists, it will be replaced with the assets being
+imported.`,
 	PreRun: func(cmd *cobra.Command, args []string) {
-		if len(args) > 0 {
-			err := cmd.Usage()
-			util.CheckErr(err)
-			os.Exit(0)
-		}
 		dockerutil.EnsureDdevNetwork()
 	},
+	Args: cobra.ExactArgs(0),
 	Run: func(cmd *cobra.Command, args []string) {
 		app, err := ddevapp.GetActiveApp("")
 		if err != nil {
 			util.Failed("Failed to import files: %v", err)
 		}
 
-		err = app.ImportFiles(fileSource, fileExtPath)
+		var showExtPathPrompt bool
+		if sourcePath == "" {
+			// Ensure we prompt for extraction path if an archive is provided, while still allowing
+			// non-interactive use of --src flag without providing a --extract-path flag.
+			if extPath == "" {
+				showExtPathPrompt = true
+			}
+
+			promptForFileSource(&sourcePath)
+		}
+
+		importPath, isArchive, err := appimport.ValidateAsset(sourcePath, "files")
 		if err != nil {
+			// Ensure we prompt for extraction path if an archive is provided, while still allowing
+			// non-interactive use of --src flag without providing a --extract-path flag.
+			if isArchive && showExtPathPrompt {
+				promptForExtPath(&extPath)
+			}
+
+			if err != nil {
+				util.Failed("Failed to import files for %s: %v", app.GetName(), err)
+			}
+		}
+
+		if err = app.ImportFiles(importPath, extPath); err != nil {
 			util.Failed("Failed to import files for %s: %v", app.GetName(), err)
 		}
+
 		util.Success("Successfully imported files for %v", app.GetName())
 	},
 }
 
+const importPathPrompt = `Provide the path to the directory or archive you wish to import.`
+
+const importPathWarn = `Please note: if the destination directory exists, it will be replaced with the
+import assets specified here.`
+
+// promptForFileSource prompts the user for the path to the source file.
+func promptForFileSource(val *string) {
+	output.UserOut.Println(importPathPrompt)
+	output.UserOut.Warnln(importPathWarn)
+
+	// An empty string isn't acceptable here, keep
+	// prompting until something is entered
+	for {
+		fmt.Print("Import path: ")
+		*val = util.GetInput("")
+		if len(*val) > 0 {
+			break
+		}
+	}
+}
+
+const extPathPrompt = `You provided an archive. Do you want to extract from a specific path in your
+archive? You may leave this blank if you wish to use the full archive contents.`
+
+// promptForExtPath prompts the user for the internal extraction path of an archive.
+func promptForExtPath(val *string) {
+	output.UserOut.Println(extPathPrompt)
+
+	// An empty string is acceptable in this case, indicating
+	// no particular extraction path
+	fmt.Print("Archive extraction path: ")
+	*val = util.GetInput("")
+}
+
 func init() {
-	ImportFileCmd.Flags().StringVarP(&fileSource, "src", "", "", "Provide the path to a directory or tar/tar.gz/tgz/zip archive of files to import")
-	ImportFileCmd.Flags().StringVarP(&fileExtPath, "extract-path", "", "", "If provided asset is an archive, provide the path to extract within the archive.")
+	ImportFileCmd.Flags().StringVarP(&sourcePath, "src", "", "", "Provide the path to a directory or tar/tar.gz/tgz/zip archive of files to import")
+	ImportFileCmd.Flags().StringVarP(&extPath, "extract-path", "", "", "If provided asset is an archive, optionally provide the path to extract within the archive.")
 	RootCmd.AddCommand(ImportFileCmd)
 }

pkg/appimport/appimport.go

@@ -12,47 +12,45 @@ import (
 	gohomedir "github.com/mitchellh/go-homedir"
 )
 
-// ValidateAsset determines if a given asset matches the required criteria for a given asset type.
-// If the path provided is a tarball, it will extract, validate, and return the extracted asset path.
-func ValidateAsset(unexpandedAssetPath string, assetType string) (string, error) {
-	var invalidAssetError = "%v. Please provide a valid asset path."
+// ValidateAsset determines if a given asset matches the required criteria for a given asset type
+// and returns the aboslute path to the asset, whether or not the asset is an archive type, and an error.
+func ValidateAsset(unexpandedAssetPath string, assetType string) (string, bool, error) {
+	var invalidAssetError = "invalid asset: %v"
 	extensions := []string{"tar", "gz", "tgz", "zip"}
 
 	// Input provided via prompt or "--flag=value" is not expanded by shell. This will help ensure ~ is expanded to the user home directory.
 	assetPath, err := gohomedir.Expand(unexpandedAssetPath)
 	if err != nil {
-		return "", fmt.Errorf(invalidAssetError, err)
+		return "", false, fmt.Errorf(invalidAssetError, err)
 	}
 
 	// ensure we are working w/ an absolute path
 	assetPath, err = filepath.Abs(assetPath)
 	if err != nil {
-		return "", fmt.Errorf(invalidAssetError, err)
+		return "", false, fmt.Errorf(invalidAssetError, err)
 	}
 
 	info, err := os.Stat(assetPath)
 	if os.IsNotExist(err) {
-		return "", fmt.Errorf(invalidAssetError, errors.New("file not found"))
+		return "", false, fmt.Errorf(invalidAssetError, errors.New("file not found"))
 	}
 	if err != nil {
-		return "", fmt.Errorf(invalidAssetError, err)
+		return "", false, fmt.Errorf(invalidAssetError, err)
 	}
 
-	// this error should not be output to user. its intent is to be evaluated by code implementing
-	// this function to handle an archive as needed.
 	for _, ext := range extensions {
 		if strings.HasSuffix(assetPath, ext) {
-			return assetPath, errors.New("is archive")
+			return assetPath, true, nil
 		}
 	}
 
 	if assetType == "files" && !info.IsDir() {
-		return "", fmt.Errorf(invalidAssetError, errors.New("provided path is not a directory or archive"))
+		return "", false, fmt.Errorf(invalidAssetError, errors.New("provided path is not a directory or archive"))
 	}
 
 	if assetType == "db" && !strings.HasSuffix(assetPath, "sql") {
-		return "", fmt.Errorf(invalidAssetError, errors.New("provided path is not a .sql file or archive"))
+		return "", false, fmt.Errorf(invalidAssetError, errors.New("provided path is not a .sql file or archive"))
 	}
 
-	return assetPath, nil
+	return assetPath, false, nil
 }

pkg/appimport/appimport_test.go

@@ -32,36 +32,38 @@ func TestValidateAsset(t *testing.T) {
 	err = os.Mkdir(testDir, 0755)
 	assert.NoError(err)
 
-	testPath, err := appimport.ValidateAsset("~/"+testDirName, "files")
+	testPath, isArchive, err := appimport.ValidateAsset("~/"+testDirName, "files")
 	assert.NoError(err)
 	assert.Contains(testPath, userDir)
 	assert.False(strings.Contains(testPath, "~"))
+	assert.False(isArchive)
 	err = os.Remove(testDir)
 	assert.NoError(err)
 
 	// test a relative path
 	deepDir := filepath.Join(testdata, "dirlevel1", "dirlevel2")
 	err = os.Chdir(deepDir)
 	assert.NoError(err)
-	testPath, err = appimport.ValidateAsset("../../dirlevel1", "files")
+	testPath, _, err = appimport.ValidateAsset("../../dirlevel1", "files")
 	assert.NoError(err)
-
 	assert.Contains(testPath, "dirlevel1")
 
 	// archive
 	testArchivePath := filepath.Join(testdata, "somedb.sql.gz")
-	_, err = appimport.ValidateAsset(testArchivePath, "db")
-	assert.Error(err)
-	assert.Equal(err.Error(), "is archive")
+	_, isArchive, err = appimport.ValidateAsset(testArchivePath, "db")
+	assert.NoError(err)
+	assert.True(isArchive)
 
 	// db no sql
 	gofilePath := filepath.Join(testdata, "junk.go")
-	_, err = appimport.ValidateAsset(gofilePath, "db")
+	_, isArchive, err = appimport.ValidateAsset(gofilePath, "db")
 	assert.Contains(err.Error(), "provided path is not a .sql file or archive")
 	assert.Error(err)
+	assert.False(isArchive)
 
 	// files not a directory
-	_, err = appimport.ValidateAsset(gofilePath, "files")
+	_, isArchive, err = appimport.ValidateAsset(gofilePath, "files")
+	assert.False(isArchive)
 	assert.Error(err)
 	assert.Contains(err.Error(), "provided path is not a directory or archive")
 }

pkg/archive/archive.go

@@ -75,6 +75,10 @@ func Untar(source string, dest string, extractionDir string) error {
 
 	defer util.CheckClose(f)
 
+	if err = os.MkdirAll(dest, 0755); err != nil {
+		return err
+	}
+
 	if strings.HasSuffix(source, "gz") {
 		gf, err := gzip.NewReader(f)
 		if err != nil {
@@ -168,6 +172,10 @@ func Unzip(source string, dest string, extractionDir string) error {
 	}
 	defer util.CheckClose(zf)
 
+	if err = os.MkdirAll(dest, 0755); err != nil {
+		return err
+	}
+
 	for _, file := range zf.File {
 		// If we have an extractionDir and this doesn't match, skip it.
 		if !strings.HasPrefix(file.Name, extractionDir) {

pkg/ddevapp/apptypes.go

@@ -34,6 +34,9 @@ type postConfigAction func(app *DdevApp) error
 // postStartAction allows actions to take place at the end of ddev start
 type postStartAction func(app *DdevApp) error
 
+// importFilesAction
+type importFilesAction func(app *DdevApp, importPath, extPath string) error
+
 // AppTypeFuncs struct defines the functions that can be called (if populated)
 // for a given appType.
 type AppTypeFuncs struct {
@@ -46,6 +49,7 @@ type AppTypeFuncs struct {
 	configOverrideAction
 	postConfigAction
 	postStartAction
+	importFilesAction
 }
 
 // appTypeMatrix is a static map that defines the various functions to be called
@@ -57,22 +61,22 @@ func init() {
 	appTypeMatrix = map[string]AppTypeFuncs{
 		"php": {},
 		"drupal6": {
-			settingsCreator: createDrupal6SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal6Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal6App, postImportDBAction: nil, configOverrideAction: drupal6ConfigOverrideAction, postConfigAction: nil, postStartAction: drupal6PostStartAction,
+			settingsCreator: createDrupal6SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal6Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal6App, postImportDBAction: nil, configOverrideAction: drupal6ConfigOverrideAction, postConfigAction: nil, postStartAction: drupal6PostStartAction, importFilesAction: drupalImportFilesAction,
 		},
 		"drupal7": {
-			settingsCreator: createDrupal7SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal7Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal7App, postImportDBAction: nil, configOverrideAction: drupal7ConfigOverrideAction, postConfigAction: nil, postStartAction: drupal7PostStartAction,
+			settingsCreator: createDrupal7SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal7Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal7App, postImportDBAction: nil, configOverrideAction: drupal7ConfigOverrideAction, postConfigAction: nil, postStartAction: drupal7PostStartAction, importFilesAction: drupalImportFilesAction,
 		},
 		"drupal8": {
-			settingsCreator: createDrupal8SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal8Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal8App, postImportDBAction: nil, configOverrideAction: nil, postConfigAction: nil, postStartAction: drupal8PostStartAction,
+			settingsCreator: createDrupal8SettingsFile, uploadDir: getDrupalUploadDir, hookDefaultComments: getDrupal8Hooks, apptypeSettingsPaths: setDrupalSiteSettingsPaths, appTypeDetect: isDrupal8App, postImportDBAction: nil, configOverrideAction: nil, postConfigAction: nil, postStartAction: drupal8PostStartAction, importFilesAction: drupalImportFilesAction,
 		},
 		"wordpress": {
-			settingsCreator: createWordpressSettingsFile, uploadDir: getWordpressUploadDir, hookDefaultComments: getWordpressHooks, apptypeSettingsPaths: setWordpressSiteSettingsPaths, appTypeDetect: isWordpressApp, postImportDBAction: wordpressPostImportDBAction, configOverrideAction: nil, postConfigAction: nil, postStartAction: nil,
+			settingsCreator: createWordpressSettingsFile, uploadDir: getWordpressUploadDir, hookDefaultComments: getWordpressHooks, apptypeSettingsPaths: setWordpressSiteSettingsPaths, appTypeDetect: isWordpressApp, postImportDBAction: wordpressPostImportDBAction, configOverrideAction: nil, postConfigAction: nil, postStartAction: nil, importFilesAction: wordpressImportFilesAction,
 		},
 		"typo3": {
-			settingsCreator: createTypo3SettingsFile, uploadDir: getTypo3UploadDir, hookDefaultComments: getTypo3Hooks, apptypeSettingsPaths: setTypo3SiteSettingsPaths, appTypeDetect: isTypo3App, postImportDBAction: nil, configOverrideAction: typo3ConfigOverrideAction, postConfigAction: nil, postStartAction: nil,
+			settingsCreator: createTypo3SettingsFile, uploadDir: getTypo3UploadDir, hookDefaultComments: getTypo3Hooks, apptypeSettingsPaths: setTypo3SiteSettingsPaths, appTypeDetect: isTypo3App, postImportDBAction: nil, configOverrideAction: typo3ConfigOverrideAction, postConfigAction: nil, postStartAction: nil, importFilesAction: typo3ImportFilesAction,
 		},
 		"backdrop": {
-			settingsCreator: createBackdropSettingsFile, uploadDir: getBackdropUploadDir, hookDefaultComments: getBackdropHooks, apptypeSettingsPaths: setBackdropSiteSettingsPaths, appTypeDetect: isBackdropApp, postImportDBAction: backdropPostImportDBAction, configOverrideAction: nil, postConfigAction: nil, postStartAction: nil,
+			settingsCreator: createBackdropSettingsFile, uploadDir: getBackdropUploadDir, hookDefaultComments: getBackdropHooks, apptypeSettingsPaths: setBackdropSiteSettingsPaths, appTypeDetect: isBackdropApp, postImportDBAction: backdropPostImportDBAction, configOverrideAction: nil, postConfigAction: nil, postStartAction: nil, importFilesAction: backdropImportFilesAction,
 		},
 	}
 }
@@ -216,3 +220,12 @@ func (app *DdevApp) PostStartAction() error {
 
 	return nil
 }
+
+// ImportFilesAction executes the relevant import files workflow for each app type.
+func (app *DdevApp) ImportFilesAction(importPath, extPath string) error {
+	if appFuncs, ok := appTypeMatrix[app.Type]; ok && appFuncs.importFilesAction != nil {
+		return appFuncs.importFilesAction(app, importPath, extPath)
+	}
+
+	return nil
+}

pkg/ddevapp/backdrop.go

@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 
 	"github.com/drud/ddev/pkg/appports"
+	"github.com/drud/ddev/pkg/archive"
 	"github.com/drud/ddev/pkg/fileutil"
 	"github.com/drud/ddev/pkg/output"
 	"github.com/drud/ddev/pkg/util"
@@ -249,3 +250,48 @@ func appendIncludeToBackdropSettingsFile(settings *BackdropSettings, siteSetting
 
 	return nil
 }
+
+// backdropImportFilesAction defines the Backdrop workflow for importing project files.
+// The Backdrop workflow is currently identical to the Drupal import-files workflow.
+func backdropImportFilesAction(app *DdevApp, importPath, extPath string) error {
+	destPath := filepath.Join(app.GetAppRoot(), app.GetDocroot(), app.GetUploadDir())
+
+	// parent of destination dir should exist
+	if !fileutil.FileExists(filepath.Dir(destPath)) {
+		return fmt.Errorf("unable to import to %s: parent directory does not exist", destPath)
+	}
+
+	// parent of destination dir should be writable.
+	if err := os.Chmod(filepath.Dir(destPath), 0755); err != nil {
+		return err
+	}
+
+	// If the destination path exists, remove it as was warned
+	if fileutil.FileExists(destPath) {
+		if err := os.RemoveAll(destPath); err != nil {
+			return fmt.Errorf("failed to cleanup %s before import: %v", destPath, err)
+		}
+	}
+
+	if isTar(importPath) {
+		if err := archive.Untar(importPath, destPath, extPath); err != nil {
+			return fmt.Errorf("failed to extract provided archive: %v", err)
+		}
+
+		return nil
+	}
+
+	if isZip(importPath) {
+		if err := archive.Unzip(importPath, destPath, extPath); err != nil {
+			return fmt.Errorf("failed to extract provided archive: %v", err)
+		}
+
+		return nil
+	}
+
+	if err := fileutil.CopyDir(importPath, destPath); err != nil {
+		return err
+	}
+
+	return nil
+}