From d48e7d1edf647db033a5288541d63ff7509191d7 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Mon, 27 Jan 2025 12:48:02 -0500 Subject: [PATCH 1/3] Added test coverage for cleanup method (#310) --- .../manager/fetcher/RemoteLspFetcherTest.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteLspFetcherTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteLspFetcherTest.java index 54ac9f3a..34c5c90d 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteLspFetcherTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteLspFetcherTest.java @@ -1,12 +1,14 @@ package software.aws.toolkits.eclipse.amazonq.lsp.manager.fetcher; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.FileNotFoundException; @@ -66,9 +68,11 @@ public final class RemoteLspFetcherTest { private static VersionRange versionRange; + private static String majorVersion; static { try { versionRange = VersionRange.createFromVersionSpec("[1.0.0, 2.0.0]"); + majorVersion = "1"; } catch (InvalidVersionSpecificationException e) { throw new AmazonQPluginException("Failed to parse LSP supported version range", e); } @@ -334,6 +338,65 @@ void fetchWhenMultipleLabelVersionsChooseLatest() throws IOException, Interrupte assertTrue(zipContentsMatchUnzipped(zipPath, unzippedPath)); } + @Test + void testCleanup() throws IOException { + + //set up compatible versions + String sampleLspVersionV2 = String.format("%s.1.0", majorVersion); + String sampleLspVersionV3 = String.format("%s.2.0", majorVersion); + sampleManifest = createManifest( + List.of( + sampleLspVersion, + createLspVersion(sampleLspVersionV2), + createLspVersion(sampleLspVersionV3)) + ); + + //create the compatible versions + Path acceptedVersion1 = Files.createDirectory(tempDir.resolve("1.7.0")); + Path acceptedVersion2 = Files.createDirectory(tempDir.resolve("1.2.0")); + + //create delisted versions + Path delistedVersion1 = Files.createDirectory(tempDir.resolve("2.0.0")); + Path delistedVersion2 = Files.createDirectory(tempDir.resolve("1.0.0")); + + //create extra version + Path extraVersion1 = Files.createDirectory(tempDir.resolve("1.1.0")); + + //verify existence of files + assertTrue(Files.exists(acceptedVersion1)); + assertTrue(Files.exists(acceptedVersion2)); + assertTrue(Files.exists(extraVersion1)); + assertTrue(Files.exists(delistedVersion1)); + assertTrue(Files.exists(delistedVersion2)); + + lspFetcher = createFetcher(); + lspFetcher.cleanup(tempDir); + + //verify compatible versions still exist + assertTrue(Files.exists(acceptedVersion1)); + assertTrue(Files.exists(acceptedVersion2)); + + //verify delisted versions were deleted + assertFalse(Files.exists(delistedVersion1)); + assertFalse(Files.exists(delistedVersion2)); + verify(mockLogger).info("Cleaning up 2 cached de-listed versions for Amazon Q Language Server"); + + //verify extra versions were deleted + assertFalse(Files.exists(extraVersion1)); + verify(mockLogger).info("Cleaning up 1 cached extra versions for Amazon Q Language Server"); + } + + @Test + void testCleanupNullManifest() throws IOException { + sampleManifest = null; + Path delistedVersion1 = Files.createDirectory(tempDir.resolve("1.0.0")); + lspFetcher = createFetcher(); + lspFetcher.cleanup(tempDir); + + assertTrue(Files.exists(tempDir)); + assertTrue(Files.exists(delistedVersion1)); + } + private HttpResponse createMockHttpResponse(final Path file, final int statusCode) { @SuppressWarnings("unchecked") HttpResponse response = mock(HttpResponse.class); From 31eba907a8289e41247f4ea8282cf883289e1a90 Mon Sep 17 00:00:00 2001 From: Jonathan Breedlove Date: Mon, 27 Jan 2025 10:58:06 -0800 Subject: [PATCH 2/3] Update syncDependencies script to fix classpath conflicts (#332) --- plugin/build/syncDependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/build/syncDependencies.sh b/plugin/build/syncDependencies.sh index d9a46cc4..2e6ce98a 100755 --- a/plugin/build/syncDependencies.sh +++ b/plugin/build/syncDependencies.sh @@ -19,7 +19,7 @@ if [ ! -f "$manifest_file" ]; then fi # Initialize the Bundle-Classpath entry -bundle_classpath="Bundle-Classpath: target/classes/,\n" +bundle_classpath="Bundle-Classpath: .,\n" # Loop through the JAR files in the dependency directory for jar in "$dependency_dir"/*.jar; do From 722fe4dc0505450afe286c6a75b659bb2be77c5e Mon Sep 17 00:00:00 2001 From: Jonathan Breedlove Date: Mon, 27 Jan 2025 11:04:19 -0800 Subject: [PATCH 3/3] Handle invalid proxy URLs (#331) --- .../connection/QLspConnectionProvider.java | 3 ++- .../preferences/AmazonQPreferencePage.java | 1 + .../eclipse/amazonq/util/Constants.java | 2 ++ .../eclipse/amazonq/util/ProxyUtil.java | 23 ++++++++++++++++ .../eclipse/amazonq/util/ProxyUtilTest.java | 26 +++++++++++++++++++ 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/connection/QLspConnectionProvider.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/connection/QLspConnectionProvider.java index 80ea5c2d..78a6556c 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/connection/QLspConnectionProvider.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/lsp/connection/QLspConnectionProvider.java @@ -18,6 +18,7 @@ import software.aws.toolkits.eclipse.amazonq.providers.LspManagerProvider; import software.aws.toolkits.eclipse.amazonq.telemetry.LanguageServerTelemetryProvider; import software.aws.toolkits.eclipse.amazonq.telemetry.metadata.ExceptionMetadata; +import software.aws.toolkits.eclipse.amazonq.util.ProxyUtil; import software.aws.toolkits.telemetry.TelemetryDefinitions.Result; import software.aws.toolkits.eclipse.amazonq.plugin.Activator; import software.aws.toolkits.eclipse.amazonq.preferences.AmazonQPreferencePage; @@ -43,7 +44,7 @@ public QLspConnectionProvider() throws IOException { @Override protected final void addEnvironmentVariables(final Map env) { - String httpsProxyPreference = Activator.getDefault().getPreferenceStore().getString(AmazonQPreferencePage.HTTPS_PROXY); + String httpsProxyPreference = ProxyUtil.getHttpsProxyUrl(); String caCertPreference = Activator.getDefault().getPreferenceStore().getString(AmazonQPreferencePage.CA_CERT); if (!StringUtils.isEmpty(httpsProxyPreference)) { env.put("HTTPS_PROXY", httpsProxyPreference); diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java index 0e1c962c..2beadb6e 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/preferences/AmazonQPreferencePage.java @@ -193,6 +193,7 @@ private void createHttpsProxyField() { addField(httpsProxy); createLabel(""" Sets the address of the proxy to use for all HTTPS connections. \ + For example "http://localhost:8888". \ Leave blank if not using a proxy. Eclipse restart required to take effect. """, 20, getFieldEditorParent()); diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java index f0a6b875..fa290cea 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/Constants.java @@ -40,5 +40,7 @@ private Constants() { public static final String AUTHENTICATE_FAILURE_MESSAGE = "An error occurred while attempting to authenticate. Please try again."; public static final String IDE_SSL_HANDSHAKE_TITLE = "SSL Handshake Error"; public static final String IDE_SSL_HANDSHAKE_BODY = "The plugin encountered an SSL handshake error. If using a proxy, check the plugin preferences."; + public static final String INVALID_PROXY_CONFIGURATION_TITLE = "Proxy Configuration Error"; + public static final String INVALID_PROXY_CONFIGURATION_BODY = "The provided proxy URL is invalid. Please check your plugin configuration."; } diff --git a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ProxyUtil.java b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ProxyUtil.java index be52afd0..a9b41a7e 100644 --- a/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ProxyUtil.java +++ b/plugin/src/software/aws/toolkits/eclipse/amazonq/util/ProxyUtil.java @@ -4,18 +4,25 @@ package software.aws.toolkits.eclipse.amazonq.util; import java.io.FileInputStream; +import java.net.URI; import java.security.KeyStore; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManagerFactory; + +import org.eclipse.mylyn.commons.ui.dialogs.AbstractNotificationPopup; +import org.eclipse.swt.widgets.Display; + import software.amazon.awssdk.utils.StringUtils; import software.aws.toolkits.eclipse.amazonq.plugin.Activator; import software.aws.toolkits.eclipse.amazonq.preferences.AmazonQPreferencePage; public final class ProxyUtil { + private static boolean hasSeenInvalidProxyNotification; + private ProxyUtil() { // Prevent initialization } @@ -30,6 +37,22 @@ protected static String getHttpsProxyUrl(final String envVarValue, final String if (!StringUtils.isEmpty(prefValue)) { httpsProxy = prefValue; } + try { + if (!StringUtils.isEmpty(httpsProxy)) { + URI.create(httpsProxy); + } + } catch (IllegalArgumentException e) { + if (!hasSeenInvalidProxyNotification) { + hasSeenInvalidProxyNotification = true; + Display.getDefault().asyncExec(() -> { + AbstractNotificationPopup notification = new ToolkitNotification(Display.getCurrent(), + Constants.INVALID_PROXY_CONFIGURATION_TITLE, + Constants.INVALID_PROXY_CONFIGURATION_BODY); + notification.open(); + }); + } + return null; + } return httpsProxy; } diff --git a/plugin/tst/software/aws/toolkits/eclipse/amazonq/util/ProxyUtilTest.java b/plugin/tst/software/aws/toolkits/eclipse/amazonq/util/ProxyUtilTest.java index d8ef582c..c9e5e983 100644 --- a/plugin/tst/software/aws/toolkits/eclipse/amazonq/util/ProxyUtilTest.java +++ b/plugin/tst/software/aws/toolkits/eclipse/amazonq/util/ProxyUtilTest.java @@ -4,7 +4,13 @@ package software.aws.toolkits.eclipse.amazonq.util; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; + +import org.eclipse.swt.widgets.Display; class ProxyUtilTest { @@ -31,4 +37,24 @@ void testPreferenceProxyUrlPrecedence() { assertEquals(mockUrl, ProxyUtil.getHttpsProxyUrl("http://bar.com:8888", mockUrl)); } + @Test + void testEnvVarInvalidProxyUrl() { + try (MockedStatic displayMock = mockStatic(Display.class)) { + Display mockDisplay = mock(Display.class); + displayMock.when(Display::getDefault).thenReturn(mockDisplay); + String mockUrl = "127.0.0.1:8000"; + assertEquals(null, ProxyUtil.getHttpsProxyUrl(mockUrl, null)); + } + } + + @Test + void testPreferenceInvalidProxyUrl() { + try (MockedStatic displayMock = mockStatic(Display.class)) { + Display mockDisplay = mock(Display.class); + displayMock.when(Display::getDefault).thenReturn(mockDisplay); + String mockUrl = "127.0.0.1:8000"; + assertEquals(null, ProxyUtil.getHttpsProxyUrl(null, mockUrl)); + } + } + }