Skip to content

Add logging and remove validation with developer profile querying #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 29, 2025
Merged

Add logging and remove validation with developer profile querying #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cab66bc
Add logging and remove validation with developer profile querying
shruti0085 Jul 28, 2025
bad131f
add null check around region
shruti0085 Jul 28, 2025
55af52e
Remove redundant region field saved to disk
shruti0085 Jul 28, 2025
2e83fd4
Parse arn to get region and add some more logs
shruti0085 Jul 29, 2025
5122de1
record error for account id parsing
shruti0085 Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions plugin/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="3.31.0",
Bundle-Classpath: .,
target/dependency/annotations.jar,
target/dependency/apache-client.jar,
target/dependency/arns.jar,
target/dependency/auth.jar,
target/dependency/aws-core.jar,
target/dependency/aws-json-protocol.jar,
Expand Down
5 changes: 5 additions & 0 deletions plugin/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@
<artifactId>regions</artifactId>
<version>${aws.java.sdk.version}</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>arns</artifactId>
<version>${aws.java.sdk.version}</version>
</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>apache-client</artifactId>
Expand Down
96 changes: 51 additions & 45 deletions ...c/software/aws/toolkits/eclipse/amazonq/configuration/profiles/QDeveloperProfileUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.eclipse.swt.widgets.Display;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;

import software.amazon.awssdk.utils.StringUtils;
Expand Down Expand Up @@ -61,51 +60,36 @@ private QDeveloperProfileUtil() { // prevent initialization
.ofNullable(Activator.getPluginStore().get(ViewConstants.Q_DEVELOPER_PROFILE_SELECTION_KEY))
.map(json -> {
try {
if (isValidSerializedProfile(json)) {
return deserializeProfile(json);
Activator.getLogger().info("Found cached developer profile during init, attempting to validate and deserialize");
QDeveloperProfile profile = deserializeProfile(json);
if (isValidCachedProfile(profile)) {
Activator.getLogger().info("Loaded cached developer profile: " + profile.getName());
return profile;
} else {
Activator.getLogger().error("Cached profile has invalid format");
Activator.getLogger().error("Cached profile has invalid data");
}
} catch (final JsonProcessingException e) {
Activator.getLogger().error("Failed to process cached profile", e);
Activator.getLogger().error("Failed to deserialize cached profile", e);
}
return null;
}).orElse(null);

} catch (Exception e) {
Activator.getLogger().error("Failed to deserialize developer profile", e);
Activator.getLogger().error("Failed to load developer profile during init", e);
}
profileSelectionTask = new CompletableFuture<>();
profiles = new ArrayList<>();
}

private boolean isValidSerializedProfile(final String profile) throws JsonProcessingException {
JsonNode node = OBJECT_MAPPER.readTree(profile);
return node.has("arn") && isValidArn(node.get("arn").asText()) && node.has("name")
&& StringUtils.isNotBlank(node.get("name").asText()) && node.has("accountId")
&& isValidAccountId(node.get("accountId").asText()) && node.has("region")
&& node.get("identityDetails").has("region")
&& isValidRegion(node.get("identityDetails").get("region").asText());
}

private QDeveloperProfile deserializeProfile(final String json) throws JsonProcessingException {
QDeveloperProfile deserializedProfile = OBJECT_MAPPER.readValue(json, QDeveloperProfile.class);

if (!isValidProfile(deserializedProfile)) {
throw new JsonProcessingException("Cached profile has invalid data") {
private static final long serialVersionUID = 1L;
};
}
return deserializedProfile;
return OBJECT_MAPPER.readValue(json, QDeveloperProfile.class);
}

private String serializeProfile(final QDeveloperProfile developerProfile) throws JsonProcessingException {
if (!isValidProfile(developerProfile)) {
throw new JsonProcessingException("Developer profile has invalid data") {
private static final long serialVersionUID = 1L;
};
if (developerProfile == null) {
throw new IllegalArgumentException("Unable to serialize null profile");
}

return OBJECT_MAPPER.writeValueAsString(developerProfile);
}

Expand All @@ -118,6 +102,7 @@ public void initialize() {
Activator.getLoginService().logout();
return null;
}).thenAccept(result -> {
Activator.getLogger().info("Fetched developer profiles, validating current customization");
CustomizationUtil.validateCurrentCustomization();
});
savedDeveloperProfile = null;
Expand All @@ -135,14 +120,19 @@ public synchronized CompletableFuture<List<QDeveloperProfile>> queryForDeveloper

private synchronized CompletableFuture<List<QDeveloperProfile>> queryForDeveloperProfilesFuture(
final boolean tryApplyCachedProfile, final boolean applyProfileUnconditionally) {
Activator.getLogger().info("Fetching Q developer profiles...");
return Activator.getLspProvider().getAmazonQServer()
.thenCompose(server -> {
GetConfigurationFromServerParams params = new GetConfigurationFromServerParams(
ExpectedResponseType.Q_DEVELOPER_PROFILE);
CompletableFuture<LspServerConfigurations<QDeveloperProfile>> response = server
.getConfigurationFromServer(params);
return response;
}).thenApply(this::processConfigurations).exceptionally(throwable -> {
}).thenApply(configurations -> {
var profiles = processConfigurations(configurations);
Activator.getLogger().info("Fetched " + profiles.size() + " Q developer profiles");
return profiles;
}).exceptionally(throwable -> {
Activator.getLogger().error("Error occurred while fetching the list of Q Developer Profile: ",
throwable);
throw new AmazonQPluginException(throwable);
Expand All @@ -155,7 +145,7 @@ public synchronized List<QDeveloperProfile> queryForDeveloperProfiles(final bool
try {
return queryForDeveloperProfilesFuture(tryApplyCachedProfile, false).get();
} catch (InterruptedException e) {
Activator.getLogger().error("Interrupted when fetching profile: ", e);
Activator.getLogger().error("Error occurred when fetching profile: ", e);
}

return new ArrayList<>();
Expand All @@ -169,22 +159,24 @@ public synchronized CompletableFuture<Void> getProfileSelectionTaskFuture() {
return profileSelectionTask;
}

private boolean isValidProfile(final QDeveloperProfile profile) {
return profile != null && StringUtils.isNotBlank(profile.getName()) && isValidAccountId(profile.getAccountId())
&& isValidArn(profile.getArn()) && isValidRegion(profile.getRegion());
private boolean isValidFetchedProfile(final QDeveloperProfile profile) {
return profile != null;
}

private boolean isValidCachedProfile(final QDeveloperProfile profile) {
return profile != null && isValidAccountId(profile.getAccountId()) && isValidArn(profile.getArn()) && isValidRegion(profile.getRegion());
}

private boolean isValidAccountId(final String accountId) {
return accountId != null && accountId.matches("^\\d{12}$");
return StringUtils.isNotBlank(accountId);
}

private boolean isValidArn(final String arn) {
return arn != null && arn.matches("^arn:aws:codewhisperer:[a-z]{2}-[a-z]+-\\d:\\d{12}:profile/[A-Z0-9]+$");
return StringUtils.isNotBlank(arn);
}

private boolean isValidRegion(final String region) {
return region != null && region
.matches("^[a-z]{2}-(central|north|south|east|west|northeast|southeast|northwest|southwest)-(\\d)$");
return StringUtils.isNotBlank(region);
}
Comment on lines 170 to 180
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why drop these validations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the other IDEs do it and clients are not expected to. We expect validation to happen on the backend side where an incorrect value should fail with an appropriate message. Also the validity checking was too regex heavy which was leading to invalid states happening on the Eclipse side.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's fine to not have these check, in most scenarios, users won't touch / modify the cached file (also that's what we expect). We just need to ensure serialize/de-serialize work fine then it should be good enough.


private List<QDeveloperProfile> handleSelectedProfile(final List<QDeveloperProfile> profiles,
Expand Down Expand Up @@ -224,6 +216,7 @@ private void setProfiles(final List<QDeveloperProfile> profiles) {
private boolean handleSingleOrNoProfile(final List<QDeveloperProfile> profiles,
final boolean tryApplyCachedProfile, final boolean applyProfileUnconditionally) {
if (!profiles.isEmpty() && tryApplyCachedProfile) {
Activator.getLogger().info("Found single developer profile, auto-selecting");
setDeveloperProfile(profiles.get(0), true, applyProfileUnconditionally);
return true;
}
Expand All @@ -240,7 +233,10 @@ private boolean handleMultipleProfiles(final List<QDeveloperProfile> profiles,
});

if (isProfileSelected && tryApplyCachedProfile) {
Activator.getLogger().info("Using cached profile: " + selectedDeveloperProfile.getName());
setDeveloperProfile(selectedDeveloperProfile, true, applyProfileUnconditionally);
} else if (!isProfileSelected) {
Activator.getLogger().warn("Cached profile not found in available profiles, user selection required");
}
}
return isProfileSelected;
Expand All @@ -250,7 +246,8 @@ private List<QDeveloperProfile> processConfigurations(
final LspServerConfigurations<QDeveloperProfile> configurations) {
return Optional.ofNullable(configurations).map(
config -> {
return config.getConfigurations().stream().filter(this::isValidProfile)
// we assume backend would return a valid profile and do not any further validations
return config.getConfigurations().stream().filter(this::isValidFetchedProfile)
.collect(Collectors.toList());
})
.orElse(Collections.emptyList());
Expand All @@ -263,9 +260,13 @@ public List<QDeveloperProfile> getDeveloperProfiles() {
}

try {
return queryForDeveloperProfiles(false);
List<QDeveloperProfile> fetchedProfiles = queryForDeveloperProfiles(false);
if (fetchedProfiles == null || fetchedProfiles.isEmpty()) {
Activator.getLogger().warn("No developer profiles available");
}
return fetchedProfiles;
} catch (Exception e) {
Activator.getLogger().error("Interupted while fetching profiles: " + e);
Activator.getLogger().error("Failed to fetch profiles: ", e);
}

return null;
Expand All @@ -283,6 +284,7 @@ private CompletableFuture<Void> setDeveloperProfile(final QDeveloperProfile deve
return CompletableFuture.completedFuture(null);
}

Activator.getLogger().info("Setting developer profile: " + developerProfile.getName());
selectedDeveloperProfile = developerProfile;
saveSelectedProfile();

Expand Down Expand Up @@ -340,22 +342,26 @@ private void saveSelectedProfile() {
}
} catch (final JsonProcessingException e) {
Activator.getLogger().error("Failed to cache Q developer profile");
} catch (Exception e) {
Activator.getLogger().error("Unexpected error while caching Q developer profile", e);
}
}

public boolean isProfileSelectionRequired() {
if (profiles == null || profiles.isEmpty()) {
try {
queryForDeveloperProfiles(false);
if (profiles == null || profiles.isEmpty()) {
Activator.getLogger().info("No Q developer profiles found");
} else if (profiles.size() == 1) {
handleSingleOrNoProfile(profiles, true, false);
}
} catch (Exception e) {
Activator.getLogger().error("Interrupted when fetching profile: ", e);
}

if (profiles.size() == 1) {
handleSingleOrNoProfile(profiles, true, false);
Activator.getLogger().error("Error occurred when fetching profile: ", e);
return false;
}
}
return profiles.size() > 1;
return profiles != null && profiles.size() > 1;
}

public QDeveloperProfile getSelectedProfile() {
Expand Down
5 changes: 4 additions & 1 deletion plugin/src/software/aws/toolkits/eclipse/amazonq/views/ChangeProfileDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package software.aws.toolkits.eclipse.amazonq.views;

import org.apache.commons.lang3.StringUtils;
import org.eclipse.jface.dialogs.Dialog;
import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.BusyIndicator;
Expand Down Expand Up @@ -399,7 +400,9 @@ protected void okPressed() {
private RadioButtonWithDescriptor createRadioButton(final Composite parent,
final QDeveloperProfile developerProfile,
final int style, final boolean isSelected) {
RadioButtonWithDescriptor button = new RadioButtonWithDescriptor(parent, developerProfile.getName(),
// names can be undefined, show placeholder value when not found
String profileName = StringUtils.isNotBlank(developerProfile.getName()) ? developerProfile.getName() : "Unnamed Profile";
RadioButtonWithDescriptor button = new RadioButtonWithDescriptor(parent, profileName,
developerProfile.getRegion(), developerProfile.getAccountId(), style);
button.setData(developerProfile);
button.addSelectionListener(() -> {
Expand Down
5 changes: 3 additions & 2 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/views/LoginViewActionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ public final void handleCommand(final ParsedCommand parsedCommand, final Browser
new LoginParams().setLoginIdcParams(loginIdcParams)).get();
if (QDeveloperProfileUtil.getInstance().isProfileSelectionRequired()) {
Map<String, Object> profilesData = new HashMap<>();
profilesData.put("profiles",
QDeveloperProfileUtil.getInstance().getDeveloperProfiles());
var profiles = QDeveloperProfileUtil.getInstance().getDeveloperProfiles();
Activator.getLogger().info("Found " + profiles.size() + " developer profiles, user selection required");
profilesData.put("profiles", profiles);
Display.getDefault().asyncExec(() -> {
browser.execute(String.format("ideClient.handleProfiles(%s)",
JSON_HANDLER.serialize(profilesData)));
Expand Down
16 changes: 6 additions & 10 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/views/model/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.google.gson.annotations.SerializedName;

import software.aws.toolkits.eclipse.amazonq.plugin.Activator;
import software.amazon.awssdk.arns.Arn;

public class Configuration {
@SerializedName("arn")
Expand Down Expand Up @@ -58,18 +59,13 @@ public final String getAccountId() {

private String extractAccountId(final String arn) {
try {
if (arn.trim().isEmpty()) {
return "";
if (arn == null || arn.trim().isEmpty()) {
return null;
}

String[] chunks = arn.split(":");

// The 5th chunk is the account id
// eg: arn:aws:codewhisperer:us-west-2:012345678901:profile/ABCDEFGHIJKL
return chunks.length < 5 ? "" : chunks[4];
return Arn.fromString(arn).accountId().orElse(null);
} catch (Exception e) {
Activator.getLogger().info(e.getMessage());
return "";
Activator.getLogger().error("Error parsing arn for account Id", e);
return null;
}
}

Expand Down
15 changes: 13 additions & 2 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/views/model/QDeveloperProfile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@

package software.aws.toolkits.eclipse.amazonq.views.model;

import software.amazon.awssdk.utils.StringUtils;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.google.gson.annotations.SerializedName;
import software.amazon.awssdk.arns.Arn;
import software.aws.toolkits.eclipse.amazonq.plugin.Activator;

@JsonIgnoreProperties(ignoreUnknown = true)
public class QDeveloperProfile extends Configuration {
Expand All @@ -31,7 +35,14 @@ public final IdentityDetails getIdentityDetails() {
}

public final String getRegion() {
return identityDetails.region();
if (identityDetails != null && !StringUtils.isBlank(identityDetails.region())) {
return identityDetails.region();
}
try {
return Arn.fromString(getArn()).region().orElse(null);
} catch (Exception e) {
Activator.getLogger().error("Error parsing arn for region", e);
return null;
}
}

}
Loading