You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hop.apache.org by ha...@apache.org on 2022/02/14 14:58:30 UTC

[hop] branch master updated: HOP-3756 Dropbox VFS Improvement - Upgrade Dropbox API to version 5.0.0 - Better error handling - Root uri with only 2 slashs Dropbox:// - Fix file upload

This is an automated email from the ASF dual-hosted git repository.

hansva pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hop.git


The following commit(s) were added to refs/heads/master by this push:
     new 35f5842  HOP-3756 Dropbox VFS Improvement - Upgrade Dropbox API to version 5.0.0 - Better error handling - Root uri with only 2 slashs Dropbox:// - Fix file upload
     new 52b5f27  Merge pull request #1360 from nadment/HOP-3752
35f5842 is described below

commit 35f584257d6bc3094aa106fdb113a1bb509047f4
Author: Nicolas Adment <na...@gmail.com>
AuthorDate: Sat Feb 12 16:50:25 2022 +0100

    HOP-3756 Dropbox VFS Improvement
    - Upgrade Dropbox API to version 5.0.0
    - Better error handling
    - Root uri with only 2 slashs Dropbox://
    - Fix file upload
---
 assemblies/plugins/tech/dropbox/pom.xml            |   2 +-
 .../modules/ROOT/pages/vfs/dropbox-vfs.adoc        |   6 +-
 plugins/tech/dropbox/pom.xml                       |   4 +-
 .../apache/hop/vfs/dropbox/DropboxFileName.java    |   8 +-
 .../hop/vfs/dropbox/DropboxFileNameParser.java     |  57 +++++++++++
 .../apache/hop/vfs/dropbox/DropboxFileObject.java  | 106 ++++++++++++---------
 .../hop/vfs/dropbox/DropboxFileProvider.java       |   8 +-
 .../hop/vfs/dropbox/config/DropboxConfig.java      |   2 -
 .../vfs/dropbox/config/DropboxConfigSingleton.java |  14 ++-
 9 files changed, 136 insertions(+), 71 deletions(-)

diff --git a/assemblies/plugins/tech/dropbox/pom.xml b/assemblies/plugins/tech/dropbox/pom.xml
index 487317e..574538f 100644
--- a/assemblies/plugins/tech/dropbox/pom.xml
+++ b/assemblies/plugins/tech/dropbox/pom.xml
@@ -35,7 +35,7 @@
     <description></description>
 
     <properties>
-        <dropbox.version>4.0.0</dropbox.version>
+        <dropbox.version>5.0.0</dropbox.version>
     </properties>
 
     <dependencies>
diff --git a/docs/hop-user-manual/modules/ROOT/pages/vfs/dropbox-vfs.adoc b/docs/hop-user-manual/modules/ROOT/pages/vfs/dropbox-vfs.adoc
index 1f10795..5ef1397 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/vfs/dropbox-vfs.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/vfs/dropbox-vfs.adoc
@@ -39,7 +39,7 @@ You need to set up an OAuth 2.0 access for Dropbox by using stored access tokens
 Give a unique name for your app.
 Then, click Create App.
 Dropbox displays the App Settings panel for the app that you created
-.. In the App Settings page, click Generated Access Token
+.. In the App Settings page, select Access token expiration to 'No expiration' and then click Generated Access Token
 .. Note the value of the access token that you created
 
 . Specify this access token in the Hop system configuration:
@@ -57,10 +57,10 @@ Once done you will see a `dropbox` entry in the central `hop-config.json` file:
 
 == Usage and testing
 
-To test if the configuration works you can simply put a small CSV file in Google Drive and then use File/Open in Hop GUI.
+To test if the configuration works you can simply put a small CSV file in Dropbox and then use File/Open in Hop GUI.
 Then you type in dropbox:// as a file location and hit enter (or click the refresh button).
 Browse to the CSV file you uploaded and open it.
 If all is configured correctly you should be able to see the content in the Hop GUI.
 
-
+NOTE: A the moment, this implementation can't upload file larger than 150 MB. 
 
diff --git a/plugins/tech/dropbox/pom.xml b/plugins/tech/dropbox/pom.xml
index c328886..79375e9 100644
--- a/plugins/tech/dropbox/pom.xml
+++ b/plugins/tech/dropbox/pom.xml
@@ -45,7 +45,7 @@
     </licenses>
 
     <properties>
-        <dropbox.version>4.0.0</dropbox.version>
+        <dropbox.version>5.0.0</dropbox.version>
     </properties>
 
     <dependencies>
@@ -61,7 +61,7 @@
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
-            <version>3.8.1</version>
+            <version>${junit.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>
diff --git a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileName.java b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileName.java
index 459bb2e..c2653e0 100644
--- a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileName.java
+++ b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileName.java
@@ -25,18 +25,18 @@ import org.apache.commons.vfs2.provider.AbstractFileName;
 /** An dropbox file name. */
 public class DropboxFileName extends AbstractFileName {
 
-  protected DropboxFileName(final String path, final FileType type) {
-    super("dropbox", path, type);
+  protected DropboxFileName(final String scheme, final String path, final FileType type) {
+    super(scheme, path, type);
   }
 
   @Override
   public FileName createName(String path, FileType type) {
-    return new DropboxFileName(path, type);
+    return new DropboxFileName(getScheme(), path, type);
   }
 
   @Override
   protected void appendRootUri(StringBuilder buffer, boolean addPassword) {
     buffer.append(getScheme());
-    buffer.append("://");
+    buffer.append(":/");
   }
 }
diff --git a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileNameParser.java b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileNameParser.java
new file mode 100644
index 0000000..5c047a1
--- /dev/null
+++ b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileNameParser.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.vfs.dropbox;
+
+import org.apache.commons.vfs2.FileName;
+import org.apache.commons.vfs2.FileSystemException;
+import org.apache.commons.vfs2.FileType;
+import org.apache.commons.vfs2.provider.AbstractFileNameParser;
+import org.apache.commons.vfs2.provider.FileNameParser;
+import org.apache.commons.vfs2.provider.UriParser;
+import org.apache.commons.vfs2.provider.VfsComponentContext;
+
+/** Custom parser for the dropbox URL */
+public class DropboxFileNameParser extends AbstractFileNameParser {
+  private static final DropboxFileNameParser INSTANCE = new DropboxFileNameParser();
+
+  private DropboxFileNameParser() {
+    super();
+  }
+
+  public static FileNameParser getInstance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public FileName parseUri(VfsComponentContext context, FileName base, String uri)
+      throws FileSystemException {
+    StringBuilder name = new StringBuilder();
+
+    String scheme = UriParser.extractScheme(context.getFileSystemManager().getSchemes(), uri, name);
+    
+    UriParser.canonicalizePath(name, 0, name.length(), this);
+
+    // Normalize separators in the path
+    UriParser.fixSeparators(name);
+
+    // Normalise the path
+    FileType fileType = UriParser.normalisePath(name);
+
+    return new DropboxFileName(scheme, name.toString(), fileType);
+  }
+}
diff --git a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileObject.java b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileObject.java
index 10bac17..37f0c60 100644
--- a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileObject.java
+++ b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileObject.java
@@ -18,37 +18,37 @@
 
 package org.apache.hop.vfs.dropbox;
 
-import com.dropbox.core.DbxException;
-import com.dropbox.core.v2.DbxClientV2;
-import com.dropbox.core.v2.files.*;
+import org.apache.commons.io.output.NullOutputStream;
 import org.apache.commons.vfs2.FileNotFoundException;
 import org.apache.commons.vfs2.FileObject;
 import org.apache.commons.vfs2.FileSystemException;
 import org.apache.commons.vfs2.FileType;
 import org.apache.commons.vfs2.provider.AbstractFileName;
 import org.apache.commons.vfs2.provider.AbstractFileObject;
-
 import java.io.FilterOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.List;
+import com.dropbox.core.DbxException;
+import com.dropbox.core.v2.DbxClientV2;
+import com.dropbox.core.v2.files.FileMetadata;
+import com.dropbox.core.v2.files.ListFolderResult;
+import com.dropbox.core.v2.files.Metadata;
+import com.dropbox.core.v2.files.UploadUploader;
 
 /** An dropbox file object. */
 public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
 
   private final DbxClientV2 client;
-  private long size;
-  private long lastModified;
+  private Metadata metadata;
 
   protected DropboxFileObject(final AbstractFileName name, final DropboxFileSystem fileSystem)
       throws FileSystemException {
     super(name, fileSystem);
 
     this.client = fileSystem.getClient();
-    this.size = 0;
-    this.lastModified = 0;
     this.injectType(FileType.IMAGINARY);
   }
 
@@ -58,6 +58,15 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
       // If root folder
       if (this.getParent() == null) {
         this.injectType(FileType.FOLDER);
+        return;
+      }
+      if (this.metadata == null) {
+        try {
+          String path = this.getName().getPath();
+          this.metadata = client.files().getMetadata(path);
+        } catch (DbxException e) {
+          // Ignore
+        }
       }
     }
   }
@@ -65,14 +74,18 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
   @Override
   protected void doDetach() throws FileSystemException {
     synchronized (getFileSystem()) {
-      this.size = 0;
-      this.lastModified = 0;
+      this.metadata = null;
     }
   }
 
   @Override
   protected FileType doGetType() throws Exception {
-    return this.getName().isFile() ? FileType.FILE : FileType.FOLDER;
+    synchronized (getFileSystem()) {
+      if (metadata == null) {
+        return FileType.IMAGINARY;
+      }
+      return (metadata instanceof FileMetadata) ? FileType.FILE : FileType.FOLDER;
+    }
   }
 
   /** Fetches the children of this folder. */
@@ -82,7 +95,8 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
       String path = this.getName().getPath();
 
       // Root path should be empty
-      if ("/".equals(path)) path = "";
+      if ("/".equals(path))
+        path = "";
 
       ListFolderResult result = client.files().listFolder(path);
       while (true) {
@@ -94,7 +108,7 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
         result = this.client.files().listFolderContinue(result.getCursor());
       }
     } catch (DbxException e) {
-      throw new FileSystemException("Error find childrens of folder ", this.getName());
+      throw new FileSystemException("vfs.provider/list-children.error", this.getName(), e);
     }
 
     return childrens;
@@ -110,18 +124,8 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
       DropboxFileObject file =
           (DropboxFileObject) fileSystem.resolveFile(metadata.getPathDisplay());
       if (file != null) {
-
         // Sets the metadata for this file object.
-        if (metadata instanceof FileMetadata) {
-          FileMetadata fileMetadata = (FileMetadata) metadata;
-          file.injectType(FileType.FILE);
-          file.lastModified = fileMetadata.getClientModified().getTime();
-          file.size = fileMetadata.getSize();
-        } else if (metadata instanceof FolderMetadata) {
-          file.injectType(FileType.FOLDER);
-          file.size = 0;
-          file.lastModified = 0;
-        }
+        file.metadata = metadata;
         result[i++] = file;
       }
     }
@@ -136,37 +140,37 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
 
   @Override
   protected void doDelete() throws Exception {
-    synchronized (getFileSystem()) {
-      String path = this.getName().getPath();
-      client.files().deleteV2(path);
-    }
+    String path = this.getName().getPath();
+    client.files().deleteV2(path);
   }
 
   @Override
   protected void doRename(FileObject newFile) throws Exception {
-    synchronized (getFileSystem()) {
-      String fromPath = this.getName().getPath();
-      String toPath = newFile.getName().getPath();
-      client.files().moveV2(fromPath, toPath);
-    }
+    String fromPath = this.getName().getPath();
+    String toPath = newFile.getName().getPath();
+    client.files().moveV2(fromPath, toPath);
   }
 
   @Override
   protected void doCreateFolder() throws Exception {
-    synchronized (getFileSystem()) {
-      String path = this.getName().getPath();
-      client.files().createFolderV2(path);
-    }
+    String path = this.getName().getPath();
+    client.files().createFolderV2(path);
   }
 
   @Override
   protected long doGetContentSize() {
-    return this.size;
+    if (metadata instanceof FileMetadata) {
+      return ((FileMetadata) metadata).getSize();
+    }
+    return 0;
   }
 
   @Override
   protected long doGetLastModifiedTime() {
-    return this.lastModified;
+    if (metadata instanceof FileMetadata) {
+      return ((FileMetadata) metadata).getClientModified().getTime();
+    }
+    return 0;
   }
 
   @Override
@@ -174,23 +178,35 @@ public class DropboxFileObject extends AbstractFileObject<DropboxFileSystem> {
     String path = this.getName().getPath();
     InputStream is = client.files().download(path).getInputStream();
     if (is == null) {
-      throw new FileNotFoundException(getName().toString());
+      throw new FileNotFoundException(getName());
     }
     return is;
   }
 
   @Override
-  protected OutputStream doGetOutputStream(boolean bAppend) throws Exception {
+  protected OutputStream doGetOutputStream(boolean append) throws Exception {
+
     String path = this.getName().getPath();
-    final UploadUploader upload = client.files().upload(path);
-    return new FilterOutputStream(upload.getOutputStream()) {
+    if (append) {
+      throw new FileSystemException("vfs.provider/write-append-not-supported.error", path);
+    }
+
+    // VFS try to create file first, so we return fake output stream to avoid conflict
+    if (this.getType() == FileType.IMAGINARY) {
+      return NullOutputStream.NULL_OUTPUT_STREAM;
+    }
+
+    // TODO: Uploader is limited to 150MB, use upload session to increase upload to 350GB.
+    final UploadUploader uploader = client.files().upload(path);
+    return new FilterOutputStream(uploader.getOutputStream()) {
       @Override
       public void close() throws IOException {
-        super.close();
         try {
-          upload.finish();
+          uploader.finish();
         } catch (Exception e) {
           throw new IOException("Failed to upload " + getName(), e);
+        } finally {
+          uploader.close();
         }
       }
     };
diff --git a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileProvider.java b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileProvider.java
index aa8383a..1e24a29 100644
--- a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileProvider.java
+++ b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/DropboxFileProvider.java
@@ -25,7 +25,6 @@ import org.apache.commons.vfs2.*;
 import org.apache.commons.vfs2.provider.AbstractOriginatingFileProvider;
 import org.apache.hop.vfs.dropbox.config.DropboxConfig;
 import org.apache.hop.vfs.dropbox.config.DropboxConfigSingleton;
-
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -48,11 +47,6 @@ public class DropboxFileProvider extends AbstractOriginatingFileProvider {
               Capability.URI,
               Capability.WRITE_CONTENT));
 
-  public static final UserAuthenticationData.Type[] AUTHENTICATOR_TYPES =
-      new UserAuthenticationData.Type[] {
-        UserAuthenticationData.USERNAME, UserAuthenticationData.PASSWORD
-      };
-
   private static FileSystemOptions defaultOptions = new FileSystemOptions();
 
   public static FileSystemOptions getDefaultFileSystemOptions() {
@@ -61,6 +55,8 @@ public class DropboxFileProvider extends AbstractOriginatingFileProvider {
 
   public DropboxFileProvider() {
     super();
+    
+    setFileNameParser(DropboxFileNameParser.getInstance());
   }
 
   @Override
diff --git a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfig.java b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfig.java
index 63bbba4..651dc36 100644
--- a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfig.java
+++ b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfig.java
@@ -20,8 +20,6 @@ package org.apache.hop.vfs.dropbox.config;
 
 public class DropboxConfig {
 
-  public static final String HOP_CONFIG_DROPBOX_CONFIG_KEY = "dropbox";
-
   private String accessToken;
 
   public DropboxConfig() {}
diff --git a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfigSingleton.java b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfigSingleton.java
index bc9cc74..e495abf 100644
--- a/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfigSingleton.java
+++ b/plugins/tech/dropbox/src/main/java/org/apache/hop/vfs/dropbox/config/DropboxConfigSingleton.java
@@ -26,6 +26,8 @@ import org.apache.hop.core.logging.LogChannel;
 
 public class DropboxConfigSingleton {
 
+  public static final String HOP_CONFIG_DROPBOX_KEY = "dropbox";
+
   private static DropboxConfigSingleton configSingleton;
 
   private DropboxConfig config;
@@ -34,7 +36,7 @@ public class DropboxConfigSingleton {
     // Load from the HopConfig store
     //
     Object configObject =
-        HopConfig.getInstance().getConfigMap().get(DropboxConfig.HOP_CONFIG_DROPBOX_CONFIG_KEY);
+        HopConfig.getInstance().getConfigMap().get(HOP_CONFIG_DROPBOX_KEY);
     if (configObject == null) {
       config = new DropboxConfig();
     } else {
@@ -45,17 +47,13 @@ public class DropboxConfigSingleton {
       } catch (Exception e) {
         LogChannel.GENERAL.logError(
             "Error reading Dropbox configuration, check property '"
-                + DropboxConfig.HOP_CONFIG_DROPBOX_CONFIG_KEY
+                + HOP_CONFIG_DROPBOX_KEY
                 + "' in the Hop config json file",
             e);
         config = new DropboxConfig();
       }
     }
-    HopConfig.getInstance().getConfigMap().put(DropboxConfig.HOP_CONFIG_DROPBOX_CONFIG_KEY, config);
-  }
-
-  public static DropboxConfigSingleton getInstance() {
-    return configSingleton;
+    HopConfig.getInstance().getConfigMap().put(HOP_CONFIG_DROPBOX_KEY, config);
   }
 
   public static DropboxConfig getConfig() {
@@ -67,7 +65,7 @@ public class DropboxConfigSingleton {
 
   public static void saveConfig() throws HopException {
     HopConfig.getInstance()
-        .saveOption(DropboxConfig.HOP_CONFIG_DROPBOX_CONFIG_KEY, configSingleton.config);
+        .saveOption(HOP_CONFIG_DROPBOX_KEY, configSingleton.config);
     HopConfig.getInstance().saveToFile();
   }
 }