tpalfy commented on code in PR #6307:
URL: https://github.com/apache/nifi/pull/6307#discussion_r965946605
##########
nifi-nar-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientService.java:
##########
@@ -27,81 +26,40 @@
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 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 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) {
Review Comment:
We could add some logging instead of swallowing the exception.
##########
nifi-nar-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientProviderService.java:
##########
@@ -116,16 +121,61 @@ public class SmbjClientProviderService extends
AbstractControllerService impleme
@Override
public SmbClientService getClient() throws IOException {
- final SmbjClientService client = new SmbjClientService(smbClient,
authenticationContext, getServiceLocation());
+ Connection connection = smbClient.connect(hostname, port);
try {
- client.connectToShare(hostname, port, shareName);
+ return connectToShare(connection);
} catch (IOException e) {
- client.forceFullyCloseConnection();
- client.connectToShare(hostname, port, shareName);
+ getLogger().debug("Closing stale connection and trying to create a
new one for share " + getServiceLocation());
+
+ closeConnection(connection);
+
+ connection = smbClient.connect(hostname, port);
+ return connectToShare(connection);
+ }
+ }
+
+ private SmbjClientService connectToShare(Connection connection) throws
IOException {
+ final Session session;
+ final Share share;
+
+ try {
+ session = connection.authenticate(authenticationContext);
+ } catch (Exception e) {
+ throw new IOException("Could not create session for share " +
getServiceLocation(), e);
+ }
+
+ try {
+ share = session.connectShare(shareName);
+ } catch (Exception e) {
+ closeSession(session);
+ throw new IOException("Could not connect to share " +
getServiceLocation(), e);
}
- return client;
+ if (!(share instanceof DiskShare)) {
+ closeSession(session);
+ throw new IllegalArgumentException("DiskShare not found. Share " +
share.getClass().getSimpleName() + " found on " + getServiceLocation());
+ }
+
+ return new SmbjClientService(session, (DiskShare) share,
getServiceLocation());
+ }
+
+ private void closeConnection(Connection connection) {
+ try {
+ if (connection != null) {
+ connection.close(true);
+ }
+ } catch (IOException ignore) {
+ }
+ }
+
+ private void closeSession(Session session) {
+ try {
+ if (session != null) {
+ session.close();
+ }
+ } catch (IOException ignore) {
Review Comment:
We could add some logging instead of swallowing the exception.
##########
nifi-nar-bundles/nifi-smb-bundle/nifi-smb-smbj-client/src/main/java/org/apache/nifi/services/smb/SmbjClientProviderService.java:
##########
@@ -116,16 +121,61 @@ public class SmbjClientProviderService extends
AbstractControllerService impleme
@Override
public SmbClientService getClient() throws IOException {
- final SmbjClientService client = new SmbjClientService(smbClient,
authenticationContext, getServiceLocation());
+ Connection connection = smbClient.connect(hostname, port);
try {
- client.connectToShare(hostname, port, shareName);
+ return connectToShare(connection);
} catch (IOException e) {
- client.forceFullyCloseConnection();
- client.connectToShare(hostname, port, shareName);
+ getLogger().debug("Closing stale connection and trying to create a
new one for share " + getServiceLocation());
+
+ closeConnection(connection);
+
+ connection = smbClient.connect(hostname, port);
+ return connectToShare(connection);
+ }
+ }
+
+ private SmbjClientService connectToShare(Connection connection) throws
IOException {
+ final Session session;
+ final Share share;
+
+ try {
+ session = connection.authenticate(authenticationContext);
+ } catch (Exception e) {
+ throw new IOException("Could not create session for share " +
getServiceLocation(), e);
+ }
+
+ try {
+ share = session.connectShare(shareName);
+ } catch (Exception e) {
+ closeSession(session);
+ throw new IOException("Could not connect to share " +
getServiceLocation(), e);
}
- return client;
+ if (!(share instanceof DiskShare)) {
+ closeSession(session);
+ throw new IllegalArgumentException("DiskShare not found. Share " +
share.getClass().getSimpleName() + " found on " + getServiceLocation());
+ }
+
+ return new SmbjClientService(session, (DiskShare) share,
getServiceLocation());
+ }
+
+ private void closeConnection(Connection connection) {
+ try {
+ if (connection != null) {
+ connection.close(true);
+ }
+ } catch (IOException ignore) {
Review Comment:
We could add some logging instead of swallowing the exception.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
