Skip to content

Commit 6dfff57

Browse files
shalom938shalom938
authored
Fix issues 152 153 (#156)
* fix-issues-152-153 Signed-off-by: shalom <[email protected]> * fix-issues-152-153 Signed-off-by: shalom <[email protected]> Signed-off-by: shalom <[email protected]>
1 parent 930ea97 commit 6dfff57

File tree

19 files changed

+245
-150
lines changed

19 files changed

+245
-150
lines changed

‎ide-common/src/main/java/org/digma/intellij/plugin/analytics/Environment.java‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,24 @@
11
package org.digma.intellij.plugin.analytics;
22

33
import com.intellij.openapi.diagnostic.Logger;
4-
import com.intellij.openapi.progress.ProgressIndicator;
5-
import com.intellij.openapi.progress.Task;
64
import com.intellij.openapi.project.Project;
75
import org.apache.commons.lang3.time.StopWatch;
6+
import org.digma.intellij.plugin.common.Backgroundable;
87
import org.digma.intellij.plugin.log.Log;
98
import org.digma.intellij.plugin.notifications.NotificationUtil;
109
import org.digma.intellij.plugin.persistence.PersistenceData;
1110
import org.digma.intellij.plugin.settings.SettingsState;
1211
import org.digma.intellij.plugin.ui.model.environment.EnvironmentsSupplier;
1312
import org.jetbrains.annotations.NotNull;
1413

15-
import javax.swing.*;
1614
import java.time.Duration;
1715
import java.time.Instant;
1816
import java.util.ArrayList;
1917
import java.util.HashSet;
2018
import java.util.List;
2119
import java.util.Objects;
2220
import java.util.concurrent.TimeUnit;
21+
import java.util.concurrent.locks.ReentrantLock;
2322

2423
public class Environment implements EnvironmentsSupplier {
2524

@@ -37,6 +36,8 @@ public class Environment implements EnvironmentsSupplier {
3736

3837
private Instant lastRefreshTimestamp = Instant.now();
3938

39+
private final ReentrantLock envChangeLock = new ReentrantLock();
40+
4041
public Environment(@NotNull Project project, @NotNull AnalyticsService analyticsService, @NotNull PersistenceData persistenceData, SettingsState settingsState) {
4142
this.project = project;
4243
this.analyticsService = analyticsService;
@@ -47,7 +48,7 @@ public Environment(@NotNull Project project, @NotNull AnalyticsService analytics
4748
//call refresh on environment when connection is lost, in some cases its necessary for some components to reset or update ui.
4849
//usually these components react to environment change events, so this will trigger an environment change if not already happened before.
4950
//if the connection lost happened during environment refresh then it may cause a second redundant event but will do no harm.
50-
project.getMessageBus().connect(project).subscribe(AnalyticsServiceConnectionEvent.ANALYTICS_SERVICE_CONNECTION_EVENT_TOPIC, new AnalyticsServiceConnectionEvent() {
51+
project.getMessageBus().connect(analyticsService).subscribe(AnalyticsServiceConnectionEvent.ANALYTICS_SERVICE_CONNECTION_EVENT_TOPIC, new AnalyticsServiceConnectionEvent() {
5152
@Override
5253
public void connectionLost() {
5354
refresh();
@@ -61,7 +62,6 @@ public void connectionGained() {
6162
}
6263

6364

64-
6565
@Override
6666
public String getCurrent() {
6767
return current;
@@ -77,21 +77,29 @@ public void setCurrent(@NotNull String newEnv) {
7777
return;
7878
}
7979

80+
81+
//changeEnvironment must run in the background, the use of envChangeLock makes sure no two threads
82+
//change environment at the same time. if user changes environments very quickly they will run one after the
83+
// other.
84+
//listeners that handle environmentChanged event in most cases need to stay on the same thread.
85+
Runnable task = () -> {
86+
envChangeLock.lock();
87+
try {
88+
changeEnvironment(newEnv);
89+
} finally {
90+
envChangeLock.unlock();
91+
}
92+
};
93+
94+
Backgroundable.ensureBackground(project, "Digma: environment changed " + newEnv, task);
95+
}
96+
97+
98+
private void changeEnvironment(@NotNull String newEnv) {
8099
var oldEnv = this.current;
81100
this.current = newEnv;
82101
persistenceData.setCurrentEnv(newEnv);
83-
84-
//todo: maybe always on background
85-
if (SwingUtilities.isEventDispatchThread()) {
86-
new Task.Backgroundable(project, "Digma: environment changed...") {
87-
@Override
88-
public void run(@NotNull ProgressIndicator indicator) {
89-
notifyEnvironmentChanged(oldEnv, newEnv);
90-
}
91-
}.queue();
92-
} else {
93-
notifyEnvironmentChanged(oldEnv, newEnv);
94-
}
102+
notifyEnvironmentChanged(oldEnv, newEnv);
95103
}
96104

97105

@@ -131,28 +139,24 @@ private void refreshOnlyEverySomeTimePassed() {
131139
private void refreshOnBackground() {
132140

133141
Log.log(LOGGER::debug, "Refreshing Environments on background thread.");
134-
new Task.Backgroundable(project, "Digma: Refreshing environments...") {
135-
@Override
136-
public void run(@NotNull ProgressIndicator indicator) {
137-
refreshEnvironments();
138-
}
139-
}.queue();
142+
143+
Backgroundable.ensureBackground(project, "Refreshing Environments on background thread.", this::refreshEnvironments);
144+
140145
}
141146

142147

143148
private boolean timeToRefresh() {
144149
//don't try to refresh to often, It's usually not necessary
145150
var now = Instant.now();
146151
Duration duration = Duration.between(lastRefreshTimestamp, now);
147-
if (duration.getSeconds() < settingsState.refreshDelay){
152+
if (duration.getSeconds() < settingsState.refreshDelay) {
148153
return false;
149154
}
150155
lastRefreshTimestamp = now;
151156
return true;
152157
}
153158

154159

155-
156160
//this method should not be called on ui threads, it may hang and cause a freeze
157161
private void refreshEnvironments() {
158162

@@ -163,7 +167,7 @@ private void refreshEnvironments() {
163167

164168
Log.log(LOGGER::debug, "Refreshing Environments list");
165169
var newEnvironments = analyticsService.getEnvironments();
166-
if (newEnvironments != null && newEnvironments.size() > 0) {
170+
if (newEnvironments != null && !newEnvironments.isEmpty()) {
167171
Log.log(LOGGER::debug, "Got environments {}", newEnvironments);
168172
} else {
169173
Log.log(LOGGER::warn, "Error loading environments: {}", newEnvironments);

‎ide-common/src/main/java/org/digma/intellij/plugin/common/Backgroundable.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package org.digma.intellij.plugin.common;
2+
3+
import com.intellij.openapi.diagnostic.Logger;
4+
import com.intellij.openapi.progress.ProgressIndicator;
5+
import com.intellij.openapi.progress.Task;
6+
import com.intellij.openapi.project.Project;
7+
import org.digma.intellij.plugin.log.Log;
8+
import org.jetbrains.annotations.NotNull;
9+
10+
import javax.swing.*;
11+
12+
public class Backgroundable {
13+
14+
private static final Logger LOGGER = Logger.getInstance(Backgroundable.class);
15+
16+
private Backgroundable() {
17+
}
18+
19+
20+
public static void ensureBackground(Project project, String name, Runnable task) {
21+
22+
Log.log(LOGGER::debug, "Request to call task {}", name);
23+
24+
if (SwingUtilities.isEventDispatchThread()) {
25+
Log.log(LOGGER::debug, "Executing task {} in background thread", name);
26+
new Task.Backgroundable(project, name) {
27+
@Override
28+
public void run(@NotNull ProgressIndicator indicator) {
29+
task.run();
30+
}
31+
}.queue();
32+
} else {
33+
Log.log(LOGGER::debug, "Executing task {} in current thread", name);
34+
task.run();
35+
}
36+
}
37+
}

‎ide-common/src/main/java/org/digma/intellij/plugin/document/DocumentInfoContainer.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ public void update(@NotNull DocumentInfo documentInfo) {
6060
loadSummaries();
6161
}
6262

63+
public void refresh() {
64+
Log.log(LOGGER::debug, "Refreshing document backend data for {}: ", psiFile.getVirtualFile());
65+
loadSummaries();
66+
}
67+
6368

6469
private void loadSummaries() {
6570

@@ -172,4 +177,5 @@ public UsageStatusResult getUsageStatusOfErrors() {
172177
public MethodInfo getMethodInfo(String id) {
173178
return documentInfo.getMethods().get(id);
174179
}
180+
175181
}

‎ide-common/src/main/java/org/digma/intellij/plugin/document/DocumentInfoService.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.util.Collections;
1717
import java.util.HashMap;
1818
import java.util.Map;
19+
import java.util.Set;
1920

2021
/**
2122
* DocumentInfoService acts as a container for DocumentInfo objects.
@@ -38,10 +39,19 @@ public DocumentInfoService(Project project) {
3839
}
3940

4041

41-
@SuppressWarnings("unused")
42+
public Set<PsiFile> allKeys() {
43+
return documents.keySet();
44+
}
45+
46+
4247
public void environmentChanged(String newEnv) {
43-
Log.log(LOGGER::debug, "Got environmentChanged event {}",newEnv);
44-
documents.clear();
48+
Log.log(LOGGER::debug, "Got environmentChanged event {}", newEnv);
49+
50+
//refresh all backend data.
51+
//must run in background
52+
documents.forEach((psiFile, container) -> {
53+
container.refresh();
54+
});
4555
}
4656

4757
public void notifyDocumentInfoChanged(PsiFile psiFile) {
@@ -53,12 +63,15 @@ public void notifyDocumentInfoChanged(PsiFile psiFile) {
5363

5464
//called after a document is analyzed for code objects
5565
public void addCodeObjects(@NotNull PsiFile psiFile, @NotNull DocumentInfo documentInfo) {
56-
Log.log(LOGGER::debug, "Adding DocumentInfo for {},{}",psiFile.getVirtualFile(),documentInfo);
57-
DocumentInfoContainer documentInfoContainer = documents.computeIfAbsent(psiFile, psiFile1 -> new DocumentInfoContainer(psiFile1,analyticsService));
66+
Log.log(LOGGER::debug, "Adding DocumentInfo for {},{}", psiFile.getVirtualFile(), documentInfo);
67+
DocumentInfoContainer documentInfoContainer = documents.computeIfAbsent(psiFile, psiFile1 -> new DocumentInfoContainer(psiFile1, analyticsService));
5868
documentInfoContainer.update(documentInfo);
5969
notifyDocumentInfoChanged(psiFile);
6070
}
6171

72+
public void removeDocumentInfo(@NotNull PsiFile psiFile) {
73+
documents.remove(psiFile);
74+
}
6275

6376
@Nullable
6477
public DocumentInfoContainer getDocumentInfo(PsiFile psiFile) {
@@ -99,4 +112,5 @@ public MethodInfo findMethodInfo(String sourceCodeObjectId) {
99112
findAny().map(documentInfoContainer -> documentInfoContainer.getMethodInfo(sourceCodeObjectId)).
100113
orElse(null);
101114
}
115+
102116
}

‎ide-common/src/main/kotlin/org/digma/intellij/plugin/analytics/BackendConnectionMonitor.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class BackendConnectionMonitor(val project: Project) : Disposable, AnalyticsServ
99

1010

1111
init {
12-
project.messageBus.connect(project)
12+
project.messageBus.connect(this)
1313
.subscribe(AnalyticsServiceConnectionEvent.ANALYTICS_SERVICE_CONNECTION_EVENT_TOPIC, this)
1414
}
1515

‎ide-common/src/main/kotlin/org/digma/intellij/plugin/ui/service/AbstractViewService.kt‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.digma.intellij.plugin.ui.service
22

3+
import com.intellij.openapi.Disposable
34
import com.intellij.openapi.project.Project
45
import com.intellij.openapi.wm.ToolWindow
56
import com.intellij.ui.content.Content
@@ -9,7 +10,7 @@ import org.digma.intellij.plugin.ui.panels.DigmaTabPanel
910
import javax.swing.SwingUtilities
1011

1112

12-
abstract class AbstractViewService(val project: Project) {
13+
abstract class AbstractViewService(val project: Project) : Disposable {
1314

1415
//these may be null if the tool window did not open yet
1516
var panel: DigmaTabPanel? = null
@@ -20,7 +21,7 @@ abstract class AbstractViewService(val project: Project) {
2021

2122
init {
2223
//subscribe to connection lost/gained , call doUpdateUi() on each event so that the no connection card will show or hide
23-
project.messageBus.connect(project)
24+
project.messageBus.connect(this)
2425
.subscribe(AnalyticsServiceConnectionEvent.ANALYTICS_SERVICE_CONNECTION_EVENT_TOPIC,
2526
handler = object : AnalyticsServiceConnectionEvent {
2627
override fun connectionLost() {
@@ -140,8 +141,11 @@ abstract class AbstractViewService(val project: Project) {
140141
}
141142
}
142143

143-
protected fun getNonSupportedFileScopeMessage(fileUri: String?): String{
144-
return NOT_SUPPORTED_OBJECT_MSG + " " +fileUri?.substringAfterLast('/',fileUri)
144+
protected fun getNonSupportedFileScopeMessage(fileUri: String?): String {
145+
return NOT_SUPPORTED_OBJECT_MSG + " " + fileUri?.substringAfterLast('/', fileUri)
145146
}
146147

148+
override fun dispose() {
149+
//do nothing
150+
}
147151
}

‎ide-common/src/main/kotlin/org/digma/intellij/plugin/ui/service/SummaryViewService.kt‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package org.digma.intellij.plugin.ui.service
33
import com.intellij.openapi.diagnostic.Logger
44
import com.intellij.openapi.project.Project
55
import org.digma.intellij.plugin.analytics.EnvironmentChanged
6+
import org.digma.intellij.plugin.common.Backgroundable
67
import org.digma.intellij.plugin.common.DumbAwareNotifier
78
import org.digma.intellij.plugin.log.Log
89
import org.digma.intellij.plugin.model.Models
@@ -35,17 +36,22 @@ class SummaryViewService(project: Project) : AbstractViewService(project) {
3536
}
3637

3738
//this is for when environment changes or connection lost and regained
38-
project.messageBus.connect(project)
39+
project.messageBus.connect(this)
3940
.subscribe(EnvironmentChanged.ENVIRONMENT_CHANGED_TOPIC, object : EnvironmentChanged {
4041

4142
override fun environmentChanged(newEnv: String?) {
4243
Log.log(logger::debug, "environmentChanged called")
43-
reload()
44+
Backgroundable.ensureBackground(project, "Summary view Reload") {
45+
reload()
46+
}
47+
4448
}
4549

4650
override fun environmentsListChanged(newEnvironments: MutableList<String>?) {
4751
Log.log(logger::debug, "environmentsListChanged called")
48-
reload()
52+
Backgroundable.ensureBackground(project, "Summary view Reload") {
53+
reload()
54+
}
4955
}
5056
})
5157
}

‎model/src/main/kotlin/org/digma/intellij/plugin/ui/model/environment/EnvironmentsSupplier.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package org.digma.intellij.plugin.ui.model.environment
22

3+
import org.jetbrains.annotations.NotNull
4+
35
interface EnvironmentsSupplier {
46
fun getEnvironments(): List<String>
5-
fun setCurrent(selectedItem: String?)
6-
fun getCurrent():String?
7+
fun setCurrent(@NotNull selectedItem: String?)
8+
fun getCurrent(): String?
79
fun refresh()
810
fun refreshNowOnBackground()
911
}

‎rider/Digma.Rider.Plugin/Digma.Rider/Protocol/CodeObjectsHost.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,7 @@ public CodeObjectsHost(Lifetime lifetime, ISolution solution,
130130
{
131131
Log(_logger, "Document '{0}' is not complete. Trying to build it on-demand.",
132132
document.FileUri);
133-
NotifyDocumentOpenedOrChanged(psiSourceFile);
134-
document = _codeObjectsCache.Map.TryGetValue(psiSourceFile);
133+
document = BuildDocumentOnDemand(psiSourceFile);
135134
}
136135

137136
if (document == null)

‎rider/src/main/java/org/digma/intellij/plugin/rider/editor/RiderEditorEventsHandler.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ public void start(@NotNull Project project) {
5858

5959

6060

61-
//todo: keep this code as example until we are sure we don't need it.
6261
fileEditorMessageBusConnection = project.getMessageBus().connect();
6362
fileEditorMessageBusConnection.subscribe(FileEditorManagerListener.FILE_EDITOR_MANAGER, new FileEditorManagerListener() {
6463
//resharper does not send an event on non-supported files,and when a non-supported file is opened our context is still shown
@@ -95,7 +94,9 @@ public void selectionChanged(@NotNull FileEditorManagerEvent event) {
9594

9695
@Override
9796
public void documentInfoChanged(PsiFile psiFile) {
98-
//there's no need to refresh ElementUnderCaret if the tool window wasn't opened yet
97+
//refresh element under caret, sometimes the document that opens does not grab focus and
98+
// element under caret event is not fired, this will cause an element under caret event.
99+
// there's no need to refresh if the tool window wasn't opened yet
99100
if (initialized) {
100101
ElementUnderCaretDetector elementUnderCaretDetector = getProject().getService(ElementUnderCaretDetector.class);
101102
elementUnderCaretDetector.refresh();

0 commit comments

Comments
 (0)