[GitHub] [nifi] turcsanyip commented on a diff in pull request #6307: NIFI-10364: Simplified connection/session handling in SmbjClientService

[GitBox <gi...@apache.org>]

turcsanyip commented on code in PR #6307:
URL: https://github.com/apache/nifi/pull/6307#discussion_r967272737


##########
nifi-nar-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientService.java:
##########
@@ -27,81 +26,45 @@
 import com.hierynomus.mssmb2.SMB2CreateOptions;
 import com.hierynomus.mssmb2.SMB2ShareAccess;
 import com.hierynomus.mssmb2.SMBApiException;
-import com.hierynomus.smbj.SMBClient;
-import com.hierynomus.smbj.auth.AuthenticationContext;
-import com.hierynomus.smbj.connection.Connection;
 import com.hierynomus.smbj.session.Session;
 import com.hierynomus.smbj.share.Directory;
 import com.hierynomus.smbj.share.DiskShare;
 import com.hierynomus.smbj.share.File;
-import com.hierynomus.smbj.share.Share;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.URI;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.stream.Stream;
 
-public class SmbjClientService implements SmbClientService {
+class SmbjClientService implements SmbClientService {
+
+    private final static Logger LOGGER = LoggerFactory.getLogger(SmbjClientService.class);
 
     private static final List<String> SPECIAL_DIRECTORIES = asList(".", "..");
     private static final long UNCATEGORISED_ERROR = -1L;
 
-    final private AuthenticationContext authenticationContext;
-    final private SMBClient smbClient;
-    final private URI serviceLocation;
-
-    private Connection connection;
-    private Session session;
-    private DiskShare share;
+    private final Session session;
+    private final DiskShare share;
+    private final URI serviceLocation;
 
-    public SmbjClientService(SMBClient smbClient, AuthenticationContext authenticationContext, URI serviceLocation) {
-        this.smbClient = smbClient;
-        this.authenticationContext = authenticationContext;
+    SmbjClientService(Session session, DiskShare share, URI serviceLocation) {
+        this.session = session;
+        this.share = share;
         this.serviceLocation = serviceLocation;
     }
 
-    public void connectToShare(String hostname, int port, String shareName) throws IOException {
-        Share share;
-        try {
-            connection = smbClient.connect(hostname, port);
-            session = connection.authenticate(authenticationContext);
-            share = session.connectShare(shareName);
-        } catch (Exception e) {
-            close();
-            throw new IOException("Could not connect to share " + format("%s:%d/%s", hostname, port, shareName), e);
-        }
-        if (share instanceof DiskShare) {
-            this.share = (DiskShare) share;
-        } else {
-            close();
-            throw new IllegalArgumentException("DiskShare not found. Share " +
-                    share.getClass().getSimpleName() + " found on " + format("%s:%d/%s", hostname, port,
-                    shareName));
-        }
-    }
-
-    public void forceFullyCloseConnection() {
-        try {
-            if (connection != null) {
-                connection.close(true);
-            }
-        } catch (IOException ignore) {
-        } finally {
-            connection = null;
-        }
-    }
-
     @Override
     public void close() {
         try {
             if (session != null) {
                 session.close();
             }
-        } catch (IOException ignore) {
-
-        } finally {
-            session = null;
+        } catch (Exception e) {
+            LOGGER.error("Could not close session to " + serviceLocation, e);

Review Comment:
   I did not know the `Throwable` unwrapping works with slf4j / logback.
   Thanks for the hint!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org