Skip to content

Commit d05bf2a

Browse files
a-orenclaude
a-oren
and
claude
committed
fix: address PR review feedback for Generate SBOM action
- Use packageManager instead of manifestName for ECOSYSTEM telemetry - Add project.isDisposed() checks in invokeLater blocks - Extract shared supportedManifestFiles list to SaUtils.SUPPORTED_MANIFEST_FILES Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1299ed1 commit d05bf2a

5 files changed

Lines changed: 45 additions & 60 deletions

File tree

‎src/main/java/org/jboss/tools/intellij/stackanalysis/GenerateSbomAction.java‎

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,27 +31,15 @@
3131
import java.nio.charset.StandardCharsets;
3232
import java.nio.file.Files;
3333
import java.nio.file.Path;
34-
import java.util.Arrays;
35-
import java.util.List;
3634

3735
import com.intellij.notification.NotificationGroupManager;
3836
import com.intellij.notification.NotificationType;
37+
import org.jboss.tools.intellij.componentanalysis.CAAnnotator;
3938
import org.jboss.tools.intellij.exhort.ApiService;
4039

4140
public class GenerateSbomAction extends AnAction {
4241
private static final Logger logger = Logger.getInstance(GenerateSbomAction.class);
4342

44-
private static final List<String> supportedManifestFiles = Arrays.asList(
45-
"pom.xml",
46-
"package.json",
47-
"go.mod",
48-
"requirements.txt",
49-
"build.gradle",
50-
"build.gradle.kts",
51-
"Cargo.toml",
52-
"pyproject.toml"
53-
);
54-
5543
@Override
5644
public @NotNull ActionUpdateThread getActionUpdateThread() {
5745
return ActionUpdateThread.BGT;
@@ -90,30 +78,37 @@ public void actionPerformed(@NotNull AnActionEvent event) {
9078
try {
9179
ApiService apiService = ApplicationManager.getApplication().getService(ApiService.class);
9280
String sbomJson = apiService.generateSbom(
81+
CAAnnotator.getPackageManager(manifestFile.getName()),
9382
manifestFile.getName(),
9483
manifestFile.getPath()
9584
);
9685

9786
Files.writeString(savePath, sbomJson, StandardCharsets.UTF_8);
9887

99-
ApplicationManager.getApplication().invokeLater(() ->
100-
NotificationGroupManager.getInstance()
101-
.getNotificationGroup("Red Hat Dependency Analytics")
102-
.createNotification(
103-
"SBOM saved to " + savePath,
104-
NotificationType.INFORMATION
105-
)
106-
.notify(project)
107-
);
88+
ApplicationManager.getApplication().invokeLater(() -> {
89+
if (project.isDisposed()) {
90+
return;
91+
}
92+
NotificationGroupManager.getInstance()
93+
.getNotificationGroup("Red Hat Dependency Analytics")
94+
.createNotification(
95+
"SBOM saved to " + savePath,
96+
NotificationType.INFORMATION
97+
)
98+
.notify(project);
99+
});
108100
} catch (Exception ex) {
109101
logger.error(ex);
110-
ApplicationManager.getApplication().invokeLater(() ->
111-
Messages.showErrorDialog(
112-
project,
113-
"SBOM generation failed: " + ex.getLocalizedMessage(),
114-
"Error"
115-
)
116-
);
102+
ApplicationManager.getApplication().invokeLater(() -> {
103+
if (project.isDisposed()) {
104+
return;
105+
}
106+
Messages.showErrorDialog(
107+
project,
108+
"SBOM generation failed: " + ex.getLocalizedMessage(),
109+
"Error"
110+
);
111+
});
117112
}
118113
});
119114
}
@@ -123,7 +118,7 @@ public void update(AnActionEvent event) {
123118
PsiFile psiFile = event.getData(CommonDataKeys.PSI_FILE);
124119
if (psiFile != null) {
125120
event.getPresentation().setEnabledAndVisible(
126-
supportedManifestFiles.contains(psiFile.getName())
121+
SaUtils.SUPPORTED_MANIFEST_FILES.contains(psiFile.getName())
127122
);
128123
} else {
129124
event.getPresentation().setEnabledAndVisible(false);

‎src/main/java/org/jboss/tools/intellij/stackanalysis/SaAction.java‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,9 @@
2828
import org.jboss.tools.intellij.report.AnalyticsReportUtils;
2929
import org.jetbrains.annotations.NotNull;
3030

31-
import java.util.Arrays;
32-
import java.util.List;
33-
3431
public class SaAction extends AnAction {
3532
private static final Logger logger = Logger.getInstance(SaAction.class);
3633

37-
private static final List<String> supportedManifestFiles = Arrays.asList(
38-
"pom.xml",
39-
"package.json",
40-
"go.mod",
41-
"requirements.txt",
42-
"build.gradle",
43-
"build.gradle.kts",
44-
"Cargo.toml",
45-
"pyproject.toml"
46-
);
47-
4834
public SaAction() {
4935

5036
}
@@ -123,7 +109,7 @@ public void update(AnActionEvent event) {
123109
// Check if file where context menu is opened is type of supported extension.
124110
// If yes then show the action for SA in menu
125111
if (psiFile != null) {
126-
event.getPresentation().setEnabledAndVisible(supportedManifestFiles
112+
event.getPresentation().setEnabledAndVisible(SaUtils.SUPPORTED_MANIFEST_FILES
127113
.contains(psiFile.getName()));
128114
} else {
129115
event.getPresentation().setEnabledAndVisible(false);

‎src/main/java/org/jboss/tools/intellij/stackanalysis/SaUtils.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,21 @@
1616
import com.intellij.openapi.vfs.VirtualFile;
1717
import org.jboss.tools.intellij.exhort.ApiService;
1818

19+
import java.util.List;
20+
1921
public class SaUtils {
2022

23+
public static final List<String> SUPPORTED_MANIFEST_FILES = List.of(
24+
"pom.xml",
25+
"package.json",
26+
"go.mod",
27+
"requirements.txt",
28+
"build.gradle",
29+
"build.gradle.kts",
30+
"Cargo.toml",
31+
"pyproject.toml"
32+
);
33+
2134
public JsonObject performSA(VirtualFile manifestFile) {
2235
// Get SA report for given manifest file.
2336
String reportLink;

‎src/test/java/org/jboss/tools/intellij/componentanalysis/pypi/PyprojectCAAnnotatorTest.java‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -413,20 +413,11 @@ public void testPep621IgnoreComment() {
413413

414414
// ── SaAction / SaUtils recognition tests ────────────────────────────────────
415415

416-
/** Verifies that SaAction.supportedManifestFiles contains pyproject.toml. */
416+
/** Verifies that SUPPORTED_MANIFEST_FILES contains pyproject.toml. */
417417
@Test
418-
public void testSaActionRecognizesPyprojectToml() throws Exception {
419-
// Given the SaAction class
420-
Field field = org.jboss.tools.intellij.stackanalysis.SaAction.class
421-
.getDeclaredField("supportedManifestFiles");
422-
field.setAccessible(true);
423-
424-
// When reading the supported manifest files list
425-
@SuppressWarnings("unchecked")
426-
List<String> manifests = (List<String>) field.get(null);
427-
428-
// Then pyproject.toml should be included
429-
assertTrue("SaAction should recognize pyproject.toml", manifests.contains("pyproject.toml"));
418+
public void testSaActionRecognizesPyprojectToml() {
419+
assertTrue("SUPPORTED_MANIFEST_FILES should contain pyproject.toml",
420+
org.jboss.tools.intellij.stackanalysis.SaUtils.SUPPORTED_MANIFEST_FILES.contains("pyproject.toml"));
430421
}
431422

432423
/** Verifies that SaUtils.determinePackageManagerName maps pyproject.toml to python. */

‎src/test/java/org/jboss/tools/intellij/exhort/ApiServiceTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public void apiInterfaceHasGenerateSbomMethod() throws NoSuchMethodException {
3535

3636
@Test
3737
public void apiServiceHasGenerateSbomMethod() throws NoSuchMethodException {
38-
Method method = ApiService.class.getMethod("generateSbom", String.class, String.class);
39-
assertNotNull("ApiService should have generateSbom(String, String) method", method);
38+
Method method = ApiService.class.getMethod("generateSbom", String.class, String.class, String.class);
39+
assertNotNull("ApiService should have generateSbom(String, String, String) method", method);
4040
assertEquals("generateSbom should return String", String.class, method.getReturnType());
4141
}
4242

0 commit comments

Comments
 (0)