Skip to content

Commit a4a95b0

Browse files
ndr-brtndr-brt
authored
fix: avoid to close client on data source closure (#470)
* fix: avoid to close client on data source closure * dependencies
1 parent c5cecd2 commit a4a95b0

File tree

3 files changed

+61
-30
lines changed

3 files changed

+61
-30
lines changed

‎DEPENDENCIES

+29-16
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
maven/mavencentral/com.fasterxml.jackson.core/jackson-annotations/2.10.3, Apache-2.0, approved, CQ21280
2-
maven/mavencentral/com.fasterxml.jackson.core/jackson-annotations/2.17.2, Apache-2.0, approved, #13672
3-
maven/mavencentral/com.fasterxml.jackson.core/jackson-core/2.17.2, , approved, #13665
4-
maven/mavencentral/com.fasterxml.jackson.core/jackson-databind/2.17.2, Apache-2.0, approved, #13671
5-
maven/mavencentral/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/2.17.2, Apache-2.0, approved, #14160
6-
maven/mavencentral/com.fasterxml.jackson/jackson-bom/2.17.2, Apache-2.0, approved, #14162
2+
maven/mavencentral/com.fasterxml.jackson.core/jackson-annotations/2.18.0, Apache-2.0, approved, #16364
3+
maven/mavencentral/com.fasterxml.jackson.core/jackson-core/2.18.0, Apache-2.0 AND MIT, approved, #16371
4+
maven/mavencentral/com.fasterxml.jackson.core/jackson-databind/2.18.0, Apache-2.0, approved, #16372
5+
maven/mavencentral/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/2.18.0, Apache-2.0, approved, #16625
6+
maven/mavencentral/com.fasterxml.jackson/jackson-bom/2.18.0, Apache-2.0, approved, #16628
77
maven/mavencentral/com.github.docker-java/docker-java-api/3.4.0, Apache-2.0, approved, clearlydefined
88
maven/mavencentral/com.github.docker-java/docker-java-transport-zerodep/3.4.0, Apache-2.0 AND (Apache-2.0 AND BSD-3-Clause), approved, #15745
99
maven/mavencentral/com.github.docker-java/docker-java-transport/3.4.0, Apache-2.0, approved, clearlydefined
1010
maven/mavencentral/com.google.code.findbugs/jsr305/3.0.2, CC-BY-2.5, approved, #15220
1111
maven/mavencentral/com.google.errorprone/error_prone_annotations/2.28.0, Apache-2.0, approved, #15107
1212
maven/mavencentral/com.google.guava/failureaccess/1.0.2, Apache-2.0, approved, CQ22654
13-
maven/mavencentral/com.google.guava/guava/33.3.0-jre, Apache-2.0 AND CC0-1.0 AND (Apache-2.0 AND CC-PDDC) AND (Apache-2.0 AND CC0-1.0), approved, #15952
13+
maven/mavencentral/com.google.guava/guava/33.3.1-jre, Apache-2.0 AND CC0-1.0 AND (Apache-2.0 AND CC-PDDC) AND (Apache-2.0 AND CC0-1.0), approved, #15952
1414
maven/mavencentral/com.google.guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava, Apache-2.0, approved, CQ22657
15-
maven/mavencentral/com.puppycrawl.tools/checkstyle/10.18.1, LGPL-2.1-or-later AND (Apache-2.0 AND LGPL-2.1-or-later) AND Apache-2.0, approved, #16060
15+
maven/mavencentral/com.google.j2objc/j2objc-annotations/3.0.0, Apache-2.0, approved, #13676
16+
maven/mavencentral/com.puppycrawl.tools/checkstyle/10.18.2, LGPL-2.1-or-later AND (Apache-2.0 AND LGPL-2.1-or-later) AND Apache-2.0, approved, #16060
1617
maven/mavencentral/com.squareup.okhttp3/okhttp-dnsoverhttps/4.12.0, Apache-2.0, approved, #11159
1718
maven/mavencentral/com.squareup.okhttp3/okhttp/4.12.0, Apache-2.0, approved, #15227
1819
maven/mavencentral/com.squareup.okhttp3/okhttp/4.9.3, Apache-2.0 AND MPL-2.0, approved, #3225
@@ -45,13 +46,13 @@ maven/mavencentral/jakarta.json/jakarta.json-api/2.1.3, EPL-2.0 OR GPL-2.0-only
4546
maven/mavencentral/jakarta.ws.rs/jakarta.ws.rs-api/4.0.0, EPL-2.0 OR GPL-2.0-only with Classpath-exception-2.0, approved, ee4j.rest
4647
maven/mavencentral/junit/junit/4.13.2, EPL-2.0, approved, CQ23636
4748
maven/mavencentral/net.bytebuddy/byte-buddy-agent/1.14.1, Apache-2.0, approved, #7164
48-
maven/mavencentral/net.bytebuddy/byte-buddy-agent/1.15.0, Apache-2.0, approved, #16009
49+
maven/mavencentral/net.bytebuddy/byte-buddy-agent/1.15.3, Apache-2.0, approved, #16009
4950
maven/mavencentral/net.bytebuddy/byte-buddy/1.14.1, Apache-2.0 AND BSD-3-Clause, approved, #7163
5051
maven/mavencentral/net.bytebuddy/byte-buddy/1.14.16, Apache-2.0 AND BSD-3-Clause, approved, #7163
5152
maven/mavencentral/net.bytebuddy/byte-buddy/1.14.18, Apache-2.0 AND BSD-3-Clause, approved, #7163
52-
maven/mavencentral/net.bytebuddy/byte-buddy/1.15.0, Apache-2.0 AND BSD-3-Clause, approved, #16008
53+
maven/mavencentral/net.bytebuddy/byte-buddy/1.15.3, Apache-2.0 AND BSD-3-Clause, approved, #16008
5354
maven/mavencentral/net.java.dev.jna/jna/5.13.0, Apache-2.0 AND LGPL-2.1-or-later, approved, #15196
54-
maven/mavencentral/net.sf.saxon/Saxon-HE/12.5, MPL-2.0-no-copyleft-exception AND (LicenseRef-scancode-proprietary-license AND MPL-2.0-no-copyleft-exception) AND (MPL-2.0-no-copyleft-exception AND X11) AND (MIT AND MPL-2.0-no-copyleft-exception) AND (MPL-1.0 AND MPL-2.0-no-copyleft-exception) AND (Apache-2.0 AND MPL-2.0-no-copyleft-exception) AND MPL-1.0, restricted, #16061
55+
maven/mavencentral/net.sf.saxon/Saxon-HE/12.5, W3C-19980720 AND MPL-2.0 AND MPL-1.0, approved, #16061
5556
maven/mavencentral/org.antlr/antlr4-runtime/4.13.2, BSD-3-Clause, approved, #10767
5657
maven/mavencentral/org.apache.commons/commons-compress/1.24.0, Apache-2.0 AND BSD-3-Clause AND bzip2-1.0.6 AND LicenseRef-Public-Domain, approved, #10368
5758
maven/mavencentral/org.apache.commons/commons-lang3/3.7, Apache-2.0, approved, clearlydefined
@@ -73,7 +74,7 @@ maven/mavencentral/org.apiguardian/apiguardian-api/1.1.2, Apache-2.0, approved,
7374
maven/mavencentral/org.assertj/assertj-core/3.26.0, Apache-2.0, approved, #14886
7475
maven/mavencentral/org.assertj/assertj-core/3.26.3, Apache-2.0, approved, #14886
7576
maven/mavencentral/org.awaitility/awaitility/4.2.1, Apache-2.0, approved, #14178
76-
maven/mavencentral/org.checkerframework/checker-qual/3.46.0, MIT, approved, clearlydefined
77+
maven/mavencentral/org.checkerframework/checker-qual/3.47.0, MIT, approved, clearlydefined
7778
maven/mavencentral/org.codehaus.plexus/plexus-classworlds/2.6.0, Apache-2.0 AND Plexus, approved, CQ22821
7879
maven/mavencentral/org.codehaus.plexus/plexus-component-annotations/2.1.0, Apache-2.0, approved, #809
7980
maven/mavencentral/org.codehaus.plexus/plexus-container-default/2.1.0, Apache-2.0, approved, clearlydefined
@@ -121,14 +122,24 @@ maven/mavencentral/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.9.10, Apache-2.0, a
121122
maven/mavencentral/org.jetbrains.kotlin/kotlin-stdlib/1.9.10, Apache-2.0, approved, #11827
122123
maven/mavencentral/org.jetbrains/annotations/13.0, Apache-2.0, approved, clearlydefined
123124
maven/mavencentral/org.jetbrains/annotations/17.0.0, Apache-2.0, approved, clearlydefined
124-
maven/mavencentral/org.jetbrains/annotations/24.1.0, Apache-2.0, approved, clearlydefined
125+
maven/mavencentral/org.jetbrains/annotations/25.0.0, Apache-2.0, approved, #16624
126+
maven/mavencentral/org.jetbrains/annotations/26.0.0, Apache-2.0, approved, #16629
125127
maven/mavencentral/org.junit.jupiter/junit-jupiter-api/5.11.0, EPL-2.0, approved, #15935
128+
maven/mavencentral/org.junit.jupiter/junit-jupiter-api/5.11.1, EPL-2.0, approved, #15935
129+
maven/mavencentral/org.junit.jupiter/junit-jupiter-api/5.11.2, EPL-2.0, approved, #15935
126130
maven/mavencentral/org.junit.jupiter/junit-jupiter-engine/5.11.0, EPL-2.0, approved, #15939
127-
maven/mavencentral/org.junit.jupiter/junit-jupiter-params/5.11.0, EPL-2.0, approved, #15940
131+
maven/mavencentral/org.junit.jupiter/junit-jupiter-engine/5.11.1, EPL-2.0, approved, #15939
132+
maven/mavencentral/org.junit.jupiter/junit-jupiter-engine/5.11.2, EPL-2.0, approved, #15939
133+
maven/mavencentral/org.junit.jupiter/junit-jupiter-params/5.11.2, EPL-2.0, approved, #15940
128134
maven/mavencentral/org.junit.platform/junit-platform-commons/1.11.0, EPL-2.0, approved, #15936
129-
maven/mavencentral/org.junit.platform/junit-platform-engine/1.11.0, EPL-2.0, approved, #15932
135+
maven/mavencentral/org.junit.platform/junit-platform-commons/1.11.1, EPL-2.0, approved, #15936
136+
maven/mavencentral/org.junit.platform/junit-platform-commons/1.11.2, EPL-2.0, approved, #15936
137+
maven/mavencentral/org.junit.platform/junit-platform-engine/1.11.1, EPL-2.0, approved, #15932
138+
maven/mavencentral/org.junit.platform/junit-platform-engine/1.11.2, EPL-2.0, approved, #15932
130139
maven/mavencentral/org.junit/junit-bom/5.11.0, EPL-2.0, approved, #16062
131-
maven/mavencentral/org.mockito/mockito-core/5.13.0, MIT, approved, clearlydefined
140+
maven/mavencentral/org.junit/junit-bom/5.11.1, EPL-2.0, approved, #16062
141+
maven/mavencentral/org.junit/junit-bom/5.11.2, EPL-2.0, approved, #16062
142+
maven/mavencentral/org.mockito/mockito-core/5.14.1, MIT AND (Apache-2.0 AND MIT) AND Apache-2.0, approved, #16375
132143
maven/mavencentral/org.mockito/mockito-core/5.2.0, MIT AND (Apache-2.0 AND MIT) AND Apache-2.0, approved, #7401
133144
maven/mavencentral/org.objenesis/objenesis/3.3, Apache-2.0, approved, clearlydefined
134145
maven/mavencentral/org.opentest4j/opentest4j/1.3.0, Apache-2.0, approved, #9713
@@ -138,8 +149,10 @@ maven/mavencentral/org.rnorth.duct-tape/duct-tape/1.0.8, MIT, approved, clearlyd
138149
maven/mavencentral/org.slf4j/slf4j-api/1.7.25, MIT, approved, CQ13368
139150
maven/mavencentral/org.slf4j/slf4j-api/1.7.30, MIT, approved, CQ13368
140151
maven/mavencentral/org.slf4j/slf4j-api/1.7.36, MIT, approved, CQ13368
141-
maven/mavencentral/org.testcontainers/junit-jupiter/1.20.1, MIT, approved, clearlydefined
152+
maven/mavencentral/org.testcontainers/junit-jupiter/1.20.1, None, restricted, #16552
153+
maven/mavencentral/org.testcontainers/junit-jupiter/1.20.2, None, restricted, #16552
142154
maven/mavencentral/org.testcontainers/testcontainers/1.20.1, MIT, approved, #15747
155+
maven/mavencentral/org.testcontainers/testcontainers/1.20.2, MIT, approved, #15747
143156
maven/mavencentral/org.xmlresolver/xmlresolver/5.2.2, Apache-2.0, approved, clearlydefined
144157
maven/mavencentral/software.amazon.awssdk/annotations/2.26.27, Apache-2.0, approved, clearlydefined
145158
maven/mavencentral/software.amazon.awssdk/apache-client/2.26.27, Apache-2.0, approved, clearlydefined

‎extensions/data-plane/data-plane-aws-s3/src/main/java/org/eclipse/edc/connector/dataplane/aws/s3/S3DataSource.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.ArrayList;
3030
import java.util.List;
3131
import java.util.function.Predicate;
32-
import java.util.stream.Collectors;
3332
import java.util.stream.Stream;
3433

3534
import static org.eclipse.edc.connector.dataplane.spi.pipeline.StreamFailure.Reason.GENERAL_ERROR;
@@ -111,7 +110,7 @@ private List<S3Object> fetchPrefixedS3Objects() {
111110

112111
var response = client.listObjectsV2(listObjectsRequest);
113112

114-
s3Objects.addAll(response.contents().stream().filter(isFile).collect(Collectors.toList()));
113+
s3Objects.addAll(response.contents().stream().filter(isFile).toList());
115114

116115
continuationToken = response.nextContinuationToken();
117116

@@ -122,7 +121,7 @@ private List<S3Object> fetchPrefixedS3Objects() {
122121

123122
@Override
124123
public void close() {
125-
client.close();
124+
126125
}
127126

128127
private static class S3Part implements Part {

‎extensions/data-plane/data-plane-aws-s3/src/test/java/org/eclipse/edc/connector/dataplane/aws/s3/S3DataSourceTest.java

+30-11
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import org.eclipse.edc.connector.dataplane.aws.s3.exceptions.S3DataSourceException;
1818
import org.eclipse.edc.connector.dataplane.spi.pipeline.DataSource;
19+
import org.junit.jupiter.api.Nested;
1920
import org.junit.jupiter.api.Test;
2021
import software.amazon.awssdk.services.s3.S3Client;
2122
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
@@ -38,7 +39,7 @@ public class S3DataSourceTest {
3839
private static final String OBJECT_NAME = "object-1";
3940
private static final String OBJECT_PREFIX = "my-prefix/";
4041
private static final String ERROR_MESSAGE = "Error message";
41-
private final S3Client s3ClientMock = mock(S3Client.class);
42+
private final S3Client s3Client = mock();
4243

4344
@Test
4445
void should_select_prefixed_objects_case_key_prefix_is_present() {
@@ -52,15 +53,15 @@ void should_select_prefixed_objects_case_key_prefix_is_present() {
5253
.bucketName(BUCKET_NAME)
5354
.objectName(OBJECT_NAME)
5455
.objectPrefix(OBJECT_PREFIX)
55-
.client(s3ClientMock)
56+
.client(s3Client)
5657
.build();
5758

58-
when(s3ClientMock.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(mockResponse);
59+
when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(mockResponse);
5960

6061
var result = s3Datasource.openPartStream();
6162

6263
assertThat(result.succeeded()).isTrue();
63-
verify(s3ClientMock, atLeastOnce()).listObjectsV2(any(ListObjectsV2Request.class));
64+
verify(s3Client, atLeastOnce()).listObjectsV2(any(ListObjectsV2Request.class));
6465
assertThat(result.getContent()).hasSize(2);
6566
}
6667

@@ -73,10 +74,10 @@ void should_fail_case_no_object_is_found() {
7374
.bucketName(BUCKET_NAME)
7475
.objectName(OBJECT_NAME)
7576
.objectPrefix(OBJECT_PREFIX)
76-
.client(s3ClientMock)
77+
.client(s3Client)
7778
.build();
7879

79-
when(s3ClientMock.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(mockResponse);
80+
when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(mockResponse);
8081

8182
var result = s3Datasource.openPartStream();
8283

@@ -91,13 +92,13 @@ void should_select_single_object_case_key_prefix_is_not_present() {
9192
.bucketName(BUCKET_NAME)
9293
.objectName(OBJECT_NAME)
9394
.objectPrefix(null)
94-
.client(s3ClientMock)
95+
.client(s3Client)
9596
.build();
9697

9798
var result = s3Datasource.openPartStream();
9899

99100
assertThat(result.succeeded()).isTrue();
100-
verify(s3ClientMock, never()).listObjectsV2(any(ListObjectsV2Request.class));
101+
verify(s3Client, never()).listObjectsV2(any(ListObjectsV2Request.class));
101102
assertThat(result.getContent()).hasSize(1);
102103
}
103104

@@ -115,16 +116,34 @@ void should_throw_datasource_exception_case_object_fetching_fails() {
115116
.bucketName(BUCKET_NAME)
116117
.objectName(OBJECT_NAME)
117118
.objectPrefix(OBJECT_PREFIX)
118-
.client(s3ClientMock)
119+
.client(s3Client)
119120
.build();
120121

121-
when(s3ClientMock.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(mockResponse);
122-
when(s3ClientMock.getObject(any(GetObjectRequest.class))).thenThrow(mockThrowable);
122+
when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(mockResponse);
123+
when(s3Client.getObject(any(GetObjectRequest.class))).thenThrow(mockThrowable);
123124

124125
var s3DataSourceException = assertThrows(S3DataSourceException.class, () ->
125126
s3Datasource.openPartStream().getContent().map(DataSource.Part::openStream).toList());
126127
assertThat(s3DataSourceException).hasCause(mockThrowable);
127128
assertThat(s3DataSourceException.getMessage()).isEqualTo(ERROR_MESSAGE);
128129
}
129130

131+
@Nested
132+
class Close {
133+
134+
@Test
135+
void shouldNotCloseClient_becauseItCouldBeReused() {
136+
var s3Datasource = S3DataSource.Builder.newInstance()
137+
.bucketName(BUCKET_NAME)
138+
.objectName(OBJECT_NAME)
139+
.objectPrefix(OBJECT_PREFIX)
140+
.client(s3Client)
141+
.build();
142+
143+
s3Datasource.close();
144+
145+
verify(s3Client, never()).close();
146+
}
147+
148+
}
130149
}

0 commit comments

Comments
 (0)