Copilot commented on code in PR #25211:
URL: https://github.com/apache/pulsar/pull/25211#discussion_r2763834975
##########
pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/MockedPackagesStorage.java:
##########
@@ -45,8 +46,13 @@ public CompletableFuture<Void> writeAsync(String path,
InputStream inputStream)
CompletableFuture<Void> future = new CompletableFuture<>();
CompletableFuture.runAsync(() -> {
try {
- byte[] bytes = new byte[inputStream.available()];
- inputStream.read(bytes);
+ ByteArrayOutputStream baos = new
java.io.ByteArrayOutputStream();
+ byte[] buffer = new byte[8192];
+ int read;
+ while ((read = inputStream.read(buffer)) > 0) {
+ baos.write(buffer, 0, read);
Review Comment:
In the read loop, using `while ((read = inputStream.read(buffer)) > 0)` is
non-idiomatic and can prematurely stop if a stream ever returns 0 (resulting in
truncated writes). Use the standard EOF check (`!= -1`) and write when `read >
0` to make the test storage robust.
```suggestion
while ((read = inputStream.read(buffer)) != -1) {
if (read > 0) {
baos.write(buffer, 0, read);
}
```
##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyPackagesUploadTest.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.pulsar.proxy.server;
+
+import static com.google.common.net.HttpHeaders.EXPECT;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+import lombok.Cleanup;
+import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
+import org.apache.pulsar.broker.authentication.AuthenticationService;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.common.configuration.PulsarConfigurationLoader;
+import org.apache.pulsar.common.util.ObjectMapperFactory;
+import
org.apache.pulsar.packages.management.core.MockedPackagesStorageProvider;
+import org.apache.pulsar.packages.management.core.common.PackageMetadata;
+import org.asynchttpclient.AsyncHttpClient;
+import org.asynchttpclient.DefaultAsyncHttpClient;
+import org.asynchttpclient.DefaultAsyncHttpClientConfig;
+import org.asynchttpclient.RequestBuilder;
+import org.asynchttpclient.Response;
+import org.asynchttpclient.request.body.multipart.FilePart;
+import org.asynchttpclient.request.body.multipart.StringPart;
+import org.eclipse.jetty.ee8.servlet.ServletHolder;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+@Test(groups = "broker-admin")
+public class ProxyPackagesUploadTest extends MockedPulsarServiceBaseTest {
+
+ private static final int FILE_SIZE = 8 * 1024 * 1024; // 8 MB
+ private static final ObjectMapper MAPPER = ObjectMapperFactory.create();
+ private WebServer webServer;
+ private PulsarAdmin proxyAdmin;
+
+ @BeforeMethod(alwaysRun = true)
+ @Override
+ protected void setup() throws Exception {
+ conf.setEnablePackagesManagement(true);
+
conf.setPackagesManagementStorageProvider(MockedPackagesStorageProvider.class.getName());
+ super.internalSetup();
+
+ ProxyConfiguration proxyConfig = new ProxyConfiguration();
+ proxyConfig.setServicePort(Optional.of(0));
+ proxyConfig.setWebServicePort(Optional.of(0));
+ proxyConfig.setBrokerWebServiceURL(brokerUrl.toString());
+
+ webServer = new WebServer(proxyConfig, new AuthenticationService(
+ PulsarConfigurationLoader.convertFrom(proxyConfig, true)));
+ webServer.addServlet("/", new ServletHolder(new
AdminProxyHandler(proxyConfig, null, null)));
+ webServer.start();
+
+ proxyAdmin = PulsarAdmin.builder()
+ .serviceHttpUrl("http://localhost:"; +
webServer.getListenPortHTTP().get())
+ .build();
+
+ admin.tenants().createTenant("public", createDefaultTenantInfo());
+ admin.namespaces().createNamespace("public/default");
+ }
+
+ @AfterMethod(alwaysRun = true)
+ @Override
+ protected void cleanup() throws Exception {
+ if (proxyAdmin != null) proxyAdmin.close();
+ if (webServer != null) webServer.stop();
+ super.internalCleanup();
+ }
+
+ @Test
+ public void testUploadPackageThroughProxy() throws Exception {
+ Path packageFile = Files.createTempFile("pkg-sdk", ".nar");
+ Files.write(packageFile, new byte[FILE_SIZE]);
+
+ String pkgName = "function://public/default/large-pkg-sdk@v1";
+ PackageMetadata meta =
PackageMetadata.builder().description("sdk-test").build();
+
+ proxyAdmin.packages().upload(meta, pkgName, packageFile.toString());
+
+ verifyDownload(pkgName, FILE_SIZE);
+
+ Files.deleteIfExists(packageFile);
+ }
+
+ @Test
+ public void testUploadWithExpect100Continue() throws Exception {
+ Path packageFile = Files.createTempFile("pkg-ahc", ".nar");
+ Files.write(packageFile, new byte[FILE_SIZE]);
+
+ String pkgName = "function://public/default/expect-test@v1";
+ String uploadUrl =
String.format("http://localhost:%d/admin/v3/packages/function/public/default/expect-test/v1";,
+ webServer.getListenPortHTTP().orElseThrow());
+
+ @Cleanup
+ AsyncHttpClient client = new DefaultAsyncHttpClient(new
DefaultAsyncHttpClientConfig.Builder().build());
+
+ Response response = client.executeRequest(new RequestBuilder("POST")
+ .setUrl(uploadUrl)
+ .addHeader(EXPECT, "100-continue")
+ .addBodyPart(new FilePart("file", packageFile.toFile()))
+ .addBodyPart(new StringPart("metadata",
MAPPER.writeValueAsString(
+
PackageMetadata.builder().description("ahc-test").build()), "application/json"))
+ .build()).get();
+
+ assertThat(response.getStatusCode()).isEqualTo(204);
+
+ verifyDownload(pkgName, FILE_SIZE);
+
+ Files.deleteIfExists(packageFile);
+ }
Review Comment:
Same cleanup issue here: `packageFile` is deleted only on the happy path.
Use a try/finally (or `deleteOnExit`) so the temp file is removed even if the
request fails or assertions throw.
##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyPackagesUploadTest.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.pulsar.proxy.server;
+
+import static com.google.common.net.HttpHeaders.EXPECT;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+import lombok.Cleanup;
+import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
+import org.apache.pulsar.broker.authentication.AuthenticationService;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.common.configuration.PulsarConfigurationLoader;
+import org.apache.pulsar.common.util.ObjectMapperFactory;
+import
org.apache.pulsar.packages.management.core.MockedPackagesStorageProvider;
+import org.apache.pulsar.packages.management.core.common.PackageMetadata;
+import org.asynchttpclient.AsyncHttpClient;
+import org.asynchttpclient.DefaultAsyncHttpClient;
+import org.asynchttpclient.DefaultAsyncHttpClientConfig;
+import org.asynchttpclient.RequestBuilder;
+import org.asynchttpclient.Response;
+import org.asynchttpclient.request.body.multipart.FilePart;
+import org.asynchttpclient.request.body.multipart.StringPart;
+import org.eclipse.jetty.ee8.servlet.ServletHolder;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+@Test(groups = "broker-admin")
+public class ProxyPackagesUploadTest extends MockedPulsarServiceBaseTest {
+
+ private static final int FILE_SIZE = 8 * 1024 * 1024; // 8 MB
+ private static final ObjectMapper MAPPER = ObjectMapperFactory.create();
+ private WebServer webServer;
+ private PulsarAdmin proxyAdmin;
+
+ @BeforeMethod(alwaysRun = true)
+ @Override
+ protected void setup() throws Exception {
+ conf.setEnablePackagesManagement(true);
+
conf.setPackagesManagementStorageProvider(MockedPackagesStorageProvider.class.getName());
+ super.internalSetup();
+
+ ProxyConfiguration proxyConfig = new ProxyConfiguration();
+ proxyConfig.setServicePort(Optional.of(0));
+ proxyConfig.setWebServicePort(Optional.of(0));
+ proxyConfig.setBrokerWebServiceURL(brokerUrl.toString());
+
+ webServer = new WebServer(proxyConfig, new AuthenticationService(
+ PulsarConfigurationLoader.convertFrom(proxyConfig, true)));
+ webServer.addServlet("/", new ServletHolder(new
AdminProxyHandler(proxyConfig, null, null)));
+ webServer.start();
+
+ proxyAdmin = PulsarAdmin.builder()
+ .serviceHttpUrl("http://localhost:"; +
webServer.getListenPortHTTP().get())
+ .build();
+
+ admin.tenants().createTenant("public", createDefaultTenantInfo());
+ admin.namespaces().createNamespace("public/default");
+ }
+
+ @AfterMethod(alwaysRun = true)
+ @Override
+ protected void cleanup() throws Exception {
+ if (proxyAdmin != null) proxyAdmin.close();
+ if (webServer != null) webServer.stop();
+ super.internalCleanup();
+ }
+
+ @Test
+ public void testUploadPackageThroughProxy() throws Exception {
+ Path packageFile = Files.createTempFile("pkg-sdk", ".nar");
+ Files.write(packageFile, new byte[FILE_SIZE]);
+
+ String pkgName = "function://public/default/large-pkg-sdk@v1";
+ PackageMetadata meta =
PackageMetadata.builder().description("sdk-test").build();
+
+ proxyAdmin.packages().upload(meta, pkgName, packageFile.toString());
+
+ verifyDownload(pkgName, FILE_SIZE);
+
+ Files.deleteIfExists(packageFile);
+ }
+
+ @Test
+ public void testUploadWithExpect100Continue() throws Exception {
+ Path packageFile = Files.createTempFile("pkg-ahc", ".nar");
+ Files.write(packageFile, new byte[FILE_SIZE]);
+
+ String pkgName = "function://public/default/expect-test@v1";
+ String uploadUrl =
String.format("http://localhost:%d/admin/v3/packages/function/public/default/expect-test/v1";,
+ webServer.getListenPortHTTP().orElseThrow());
+
+ @Cleanup
+ AsyncHttpClient client = new DefaultAsyncHttpClient(new
DefaultAsyncHttpClientConfig.Builder().build());
+
+ Response response = client.executeRequest(new RequestBuilder("POST")
+ .setUrl(uploadUrl)
+ .addHeader(EXPECT, "100-continue")
+ .addBodyPart(new FilePart("file", packageFile.toFile()))
+ .addBodyPart(new StringPart("metadata",
MAPPER.writeValueAsString(
+
PackageMetadata.builder().description("ahc-test").build()), "application/json"))
+ .build()).get();
+
+ assertThat(response.getStatusCode()).isEqualTo(204);
+
+ verifyDownload(pkgName, FILE_SIZE);
+
+ Files.deleteIfExists(packageFile);
+ }
+
+ private void verifyDownload(String packageName, int expectedSize) throws
Exception {
+ Path fromBroker = Files.createTempFile("from-broker", ".nar");
+ admin.packages().download(packageName, fromBroker.toString());
+ assertThat(Files.size(fromBroker)).isEqualTo(expectedSize);
+ Files.deleteIfExists(fromBroker);
+
+ Path fromProxy = Files.createTempFile("from-proxy", ".nar");
+ proxyAdmin.packages().download(packageName, fromProxy.toString());
+ assertThat(Files.size(fromProxy)).isEqualTo(expectedSize);
+ Files.deleteIfExists(fromProxy);
Review Comment:
Temp files are only deleted at the end of the test method. If an assertion
fails or an exception is thrown earlier, the temp file will be left behind.
Consider wrapping the body in a try/finally (or using `deleteOnExit`) to
guarantee cleanup.
```suggestion
try {
String pkgName = "function://public/default/large-pkg-sdk@v1";
PackageMetadata meta =
PackageMetadata.builder().description("sdk-test").build();
proxyAdmin.packages().upload(meta, pkgName,
packageFile.toString());
verifyDownload(pkgName, FILE_SIZE);
} finally {
Files.deleteIfExists(packageFile);
}
}
@Test
public void testUploadWithExpect100Continue() throws Exception {
Path packageFile = Files.createTempFile("pkg-ahc", ".nar");
Files.write(packageFile, new byte[FILE_SIZE]);
try {
String pkgName = "function://public/default/expect-test@v1";
String uploadUrl =
String.format("http://localhost:%d/admin/v3/packages/function/public/default/expect-test/v1";,
webServer.getListenPortHTTP().orElseThrow());
@Cleanup
AsyncHttpClient client = new DefaultAsyncHttpClient(new
DefaultAsyncHttpClientConfig.Builder().build());
Response response = client.executeRequest(new
RequestBuilder("POST")
.setUrl(uploadUrl)
.addHeader(EXPECT, "100-continue")
.addBodyPart(new FilePart("file", packageFile.toFile()))
.addBodyPart(new StringPart("metadata",
MAPPER.writeValueAsString(
PackageMetadata.builder().description("ahc-test").build()), "application/json"))
.build()).get();
assertThat(response.getStatusCode()).isEqualTo(204);
verifyDownload(pkgName, FILE_SIZE);
} finally {
Files.deleteIfExists(packageFile);
}
}
private void verifyDownload(String packageName, int expectedSize) throws
Exception {
Path fromBroker = Files.createTempFile("from-broker", ".nar");
try {
admin.packages().download(packageName, fromBroker.toString());
assertThat(Files.size(fromBroker)).isEqualTo(expectedSize);
} finally {
Files.deleteIfExists(fromBroker);
}
Path fromProxy = Files.createTempFile("from-proxy", ".nar");
try {
proxyAdmin.packages().download(packageName,
fromProxy.toString());
assertThat(Files.size(fromProxy)).isEqualTo(expectedSize);
} finally {
Files.deleteIfExists(fromProxy);
}
```
##########
pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/MockedPackagesStorageTest.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.pulsar.packages.management.core;
+
+import static org.mockito.Mockito.mock;
+import static org.testng.Assert.assertEquals;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import lombok.Cleanup;
+import org.testng.annotations.Test;
+
+public class MockedPackagesStorageTest {
+
+ @Test
+ public void testWriteAndRead() throws Exception {
+ PackagesStorageProvider provider = new MockedPackagesStorageProvider();
+ PackagesStorage storage =
provider.getStorage(mock(PackagesStorageConfiguration.class));
+ storage.initialize();
+
+ // Test data
+ byte[] testBytes = new byte[1 * 1024 * 1024];
+
+ // Write
+ @Cleanup
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ storage.writeAsync("test/path", new ByteArrayInputStream(testBytes))
+ .thenCompose(v -> storage.readAsync("test/path", baos))
+ .get();
Review Comment:
This `baos` is never asserted/used, and the chained `readAsync(..., baos)`
duplicates the explicit read that happens immediately after. Either assert on
`baos` and drop the second read, or remove this first
`ByteArrayOutputStream`/read to keep the test focused.
```suggestion
storage.writeAsync("test/path", new
ByteArrayInputStream(testBytes)).get();
```
##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyPackagesUploadTest.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.pulsar.proxy.server;
+
+import static com.google.common.net.HttpHeaders.EXPECT;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+import lombok.Cleanup;
+import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
+import org.apache.pulsar.broker.authentication.AuthenticationService;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.common.configuration.PulsarConfigurationLoader;
+import org.apache.pulsar.common.util.ObjectMapperFactory;
+import
org.apache.pulsar.packages.management.core.MockedPackagesStorageProvider;
+import org.apache.pulsar.packages.management.core.common.PackageMetadata;
+import org.asynchttpclient.AsyncHttpClient;
+import org.asynchttpclient.DefaultAsyncHttpClient;
+import org.asynchttpclient.DefaultAsyncHttpClientConfig;
+import org.asynchttpclient.RequestBuilder;
+import org.asynchttpclient.Response;
+import org.asynchttpclient.request.body.multipart.FilePart;
+import org.asynchttpclient.request.body.multipart.StringPart;
+import org.eclipse.jetty.ee8.servlet.ServletHolder;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+@Test(groups = "broker-admin")
+public class ProxyPackagesUploadTest extends MockedPulsarServiceBaseTest {
+
+ private static final int FILE_SIZE = 8 * 1024 * 1024; // 8 MB
+ private static final ObjectMapper MAPPER = ObjectMapperFactory.create();
+ private WebServer webServer;
+ private PulsarAdmin proxyAdmin;
+
+ @BeforeMethod(alwaysRun = true)
+ @Override
+ protected void setup() throws Exception {
+ conf.setEnablePackagesManagement(true);
+
conf.setPackagesManagementStorageProvider(MockedPackagesStorageProvider.class.getName());
+ super.internalSetup();
+
+ ProxyConfiguration proxyConfig = new ProxyConfiguration();
+ proxyConfig.setServicePort(Optional.of(0));
+ proxyConfig.setWebServicePort(Optional.of(0));
+ proxyConfig.setBrokerWebServiceURL(brokerUrl.toString());
+
+ webServer = new WebServer(proxyConfig, new AuthenticationService(
+ PulsarConfigurationLoader.convertFrom(proxyConfig, true)));
+ webServer.addServlet("/", new ServletHolder(new
AdminProxyHandler(proxyConfig, null, null)));
+ webServer.start();
+
+ proxyAdmin = PulsarAdmin.builder()
+ .serviceHttpUrl("http://localhost:"; +
webServer.getListenPortHTTP().get())
+ .build();
+
+ admin.tenants().createTenant("public", createDefaultTenantInfo());
+ admin.namespaces().createNamespace("public/default");
+ }
+
+ @AfterMethod(alwaysRun = true)
+ @Override
+ protected void cleanup() throws Exception {
+ if (proxyAdmin != null) proxyAdmin.close();
+ if (webServer != null) webServer.stop();
+ super.internalCleanup();
+ }
+
+ @Test
+ public void testUploadPackageThroughProxy() throws Exception {
+ Path packageFile = Files.createTempFile("pkg-sdk", ".nar");
+ Files.write(packageFile, new byte[FILE_SIZE]);
+
+ String pkgName = "function://public/default/large-pkg-sdk@v1";
+ PackageMetadata meta =
PackageMetadata.builder().description("sdk-test").build();
+
+ proxyAdmin.packages().upload(meta, pkgName, packageFile.toString());
+
+ verifyDownload(pkgName, FILE_SIZE);
+
+ Files.deleteIfExists(packageFile);
+ }
+
+ @Test
+ public void testUploadWithExpect100Continue() throws Exception {
+ Path packageFile = Files.createTempFile("pkg-ahc", ".nar");
+ Files.write(packageFile, new byte[FILE_SIZE]);
+
+ String pkgName = "function://public/default/expect-test@v1";
+ String uploadUrl =
String.format("http://localhost:%d/admin/v3/packages/function/public/default/expect-test/v1";,
+ webServer.getListenPortHTTP().orElseThrow());
+
+ @Cleanup
+ AsyncHttpClient client = new DefaultAsyncHttpClient(new
DefaultAsyncHttpClientConfig.Builder().build());
+
+ Response response = client.executeRequest(new RequestBuilder("POST")
+ .setUrl(uploadUrl)
+ .addHeader(EXPECT, "100-continue")
+ .addBodyPart(new FilePart("file", packageFile.toFile()))
+ .addBodyPart(new StringPart("metadata",
MAPPER.writeValueAsString(
+
PackageMetadata.builder().description("ahc-test").build()), "application/json"))
+ .build()).get();
+
+ assertThat(response.getStatusCode()).isEqualTo(204);
+
+ verifyDownload(pkgName, FILE_SIZE);
+
+ Files.deleteIfExists(packageFile);
+ }
+
+ private void verifyDownload(String packageName, int expectedSize) throws
Exception {
+ Path fromBroker = Files.createTempFile("from-broker", ".nar");
+ admin.packages().download(packageName, fromBroker.toString());
+ assertThat(Files.size(fromBroker)).isEqualTo(expectedSize);
+ Files.deleteIfExists(fromBroker);
+
+ Path fromProxy = Files.createTempFile("from-proxy", ".nar");
+ proxyAdmin.packages().download(packageName, fromProxy.toString());
+ assertThat(Files.size(fromProxy)).isEqualTo(expectedSize);
+ Files.deleteIfExists(fromProxy);
Review Comment:
`verifyDownload` creates temp files and deletes them on the happy path only.
If `download` or the size assertion fails, the temp files will remain. Prefer
try/finally around each temp file (or `deleteOnExit`) to ensure cleanup.
```suggestion
try {
admin.packages().download(packageName, fromBroker.toString());
assertThat(Files.size(fromBroker)).isEqualTo(expectedSize);
} finally {
Files.deleteIfExists(fromBroker);
}
Path fromProxy = Files.createTempFile("from-proxy", ".nar");
try {
proxyAdmin.packages().download(packageName,
fromProxy.toString());
assertThat(Files.size(fromProxy)).isEqualTo(expectedSize);
} finally {
Files.deleteIfExists(fromProxy);
}
```
##########
pulsar-package-management/core/src/test/java/org/apache/pulsar/packages/management/core/MockedPackagesStorage.java:
##########
@@ -45,8 +46,13 @@ public CompletableFuture<Void> writeAsync(String path,
InputStream inputStream)
CompletableFuture<Void> future = new CompletableFuture<>();
CompletableFuture.runAsync(() -> {
try {
- byte[] bytes = new byte[inputStream.available()];
- inputStream.read(bytes);
+ ByteArrayOutputStream baos = new
java.io.ByteArrayOutputStream();
+ byte[] buffer = new byte[8192];
Review Comment:
`ByteArrayOutputStream` is already imported, but this line still uses the
fully-qualified `new java.io.ByteArrayOutputStream()`. Use the imported type to
keep the code consistent and avoid redundant qualification.
--
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]
