Skip to content

Commit 94c0b0f

Browse files
mbienmbien
authored
Merge pull request #8171 from mbien/fix-ergonomics-pg-npe
Fix NPE during project group creation caused by ergonomics
2 parents b1c24b2 + 99ea64d commit 94c0b0f

File tree

3 files changed

+37
-42
lines changed

3 files changed

+37
-42
lines changed

‎ide/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,12 +234,9 @@ public Future<Project[]> openProjectsAPI() {
234234
}
235235

236236
final Project unwrapProject(Project wrap) {
237-
Project[] now = getOpenProjects();
238-
239237
if (wrap instanceof LazyProject) {
240-
LazyProject lp = (LazyProject)wrap;
241-
for (Project p : now) {
242-
if (lp.getProjectDirectory().equals(p.getProjectDirectory())) {
238+
for (Project p : getOpenProjects()) {
239+
if (wrap.getProjectDirectory().equals(p.getProjectDirectory())) {
243240
return p;
244241
}
245242
}
@@ -1379,6 +1376,7 @@ private boolean doOpenProject(final @NonNull Project p) {
13791376
public @Override Boolean run() {
13801377
log(Level.FINER, "already opened: {0} ", openProjects);
13811378
for (Project existing : openProjects) {
1379+
// TODO An old hack due to broken equals() contract; see https://bz.apache.org/netbeans/show_bug.cgi?id=156536
13821380
if (p.equals(existing) || existing.equals(p)) {
13831381
alreadyOpen.set(true);
13841382
return false;

‎ide/projectui/src/org/netbeans/modules/project/ui/ProjectUtilities.java‎

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@
7474
*/
7575
public class ProjectUtilities {
7676

77+
private static final Logger LOG = Logger.getLogger(ProjectUtilities.class.getName());
78+
7779
static final String OPEN_FILES_NS = "http://www.netbeans.org/ns/projectui-open-files/1"; // NOI18N
7880
static final String OPEN_FILES_NS2 = "http://www.netbeans.org/ns/projectui-open-files/2"; // NOI18N
7981
static final String OPEN_FILES_ELEMENT = "open-files"; // NOI18N
@@ -103,30 +105,23 @@ public boolean open (FileObject fo) {
103105
} else if (o != null) {
104106
o.open();
105107
} else {
106-
ERR.log(Level.INFO, "No EditCookie nor OpenCookie nor Openable for {0}", dobj);
108+
LOG.log(Level.INFO, "No EditCookie nor OpenCookie nor Openable for {0}", dobj);
107109
return false;
108110
}
109111
return true;
110112
}
111113

112114
@Override
113-
public Map<Project,Set<String>> close(final Project[] projects,
114-
final boolean notifyUI) {
115-
final Wrapper wr = new Wrapper();
116-
wr.urls4project = new LinkedHashMap<>();
117-
doClose(projects, notifyUI, wr);
118-
return wr.urls4project;
119-
}
120-
121-
private void doClose(Project[] projects, boolean notifyUI, Wrapper wr) {
115+
public Map<Project, Set<String>> close(Project[] projects, boolean notifyUI) {
116+
Map<Project, Set<String>> project2FilesMap = new LinkedHashMap<>();
122117
List<Project> listOfProjects = Arrays.asList(projects);
123118
for (Project p : listOfProjects) { //#232668 all projects need an entry in the map - to handle projects without files correctly
124-
wr.urls4project.put(p, new LinkedHashSet<>());
119+
project2FilesMap.put(p, new LinkedHashSet<>());
125120
}
126121
Set<DataObject> openFiles = new LinkedHashSet<>();
127122
List<TopComponent> tc2close = new ArrayList<>();
128123

129-
ERR.finer("Closing TCs");
124+
LOG.finer("Closing TCs");
130125
List<TopComponent> openedTC = getOpenedTCs();
131126

132127
for (TopComponent tc : openedTC) {
@@ -135,28 +130,23 @@ private void doClose(Project[] projects, boolean notifyUI, Wrapper wr) {
135130
if (dobj != null) {
136131
FileObject fobj = dobj.getPrimaryFile();
137132
Project owner = ProjectConvertors.getNonConvertorOwner(fobj);
138-
ERR.log(Level.FINER, "Found {0} owned by {1} in {2} of {3}", new Object[] {fobj, owner, tc.getName(), tc.getClass()});
133+
LOG.log(Level.FINER, "Found {0} owned by {1} in {2} of {3}", new Object[] {fobj, owner, tc.getName(), tc.getClass()});
139134

140-
if (listOfProjects.contains(owner)) {
135+
Set<String> files = project2FilesMap.get(owner);
136+
if (files != null) {
141137
if (notifyUI) {
142138
openFiles.add(dobj);
143139
tc2close.add(tc);
144140
} else if (!dobj.isModified()) {
145141
// when not called from UI, only include TCs that arenot modified
146142
tc2close.add(tc);
147143
}
148-
//#235897 split a single line to detect NPE better
149-
final Set<String> pwnr = wr.urls4project.get(owner);
150-
assert pwnr != null : "Owner project for file:" + fobj + " prj:" + owner;
151-
final FileObject pf = fobj;
152-
assert pf != null;
153-
URL u = pf.toURL();
154-
assert u != null;
155-
String uex = u.toExternalForm();
156-
pwnr.add(uex);
144+
files.add(fobj.toURL().toExternalForm());
145+
} else if (owner != null) {
146+
LOG.log(Level.WARNING, "project association lost, project ({0}) might lose an opened file ({1}) on reopen", new Object[] {owner, fobj});
157147
}
158148
} else {
159-
ERR.log(Level.FINE, "#194243: no DataObject in lookup of {0} of {1}", new Object[] {tc.getName(), tc.getClass()});
149+
LOG.log(Level.FINE, "#194243: no DataObject in lookup of {0} of {1}", new Object[] {tc.getName(), tc.getClass()});
160150
}
161151
}
162152
if (notifyUI) {
@@ -189,9 +179,10 @@ public void run() {
189179
} else {
190180
// signal that close was vetoed
191181
if (!openFiles.isEmpty()) {
192-
wr.urls4project = null;
182+
return null;
193183
}
194184
}
185+
return project2FilesMap;
195186
}
196187

197188
private List<TopComponent> getOpenedTCs() {
@@ -203,7 +194,7 @@ private List<TopComponent> getOpenedTCs() {
203194
if (!wm.isEditorMode(mode)) {
204195
continue;
205196
}
206-
ERR.log(Level.FINER, "Closing TCs in mode {0}", mode.getName());
197+
LOG.log(Level.FINER, "Closing TCs in mode {0}", mode.getName());
207198
openedTC.addAll(Arrays.asList(wm.getOpenedTopComponents(mode)));
208199
}
209200
};
@@ -220,12 +211,6 @@ private List<TopComponent> getOpenedTCs() {
220211
}
221212
};
222213

223-
private static class Wrapper {
224-
Map<Project,Set<String>> urls4project;
225-
}
226-
227-
private static final Logger ERR = Logger.getLogger(ProjectUtilities.class.getName());
228-
229214
private ProjectUtilities() {}
230215

231216
public static void selectAndExpandProject( final Project p ) {
@@ -541,12 +526,12 @@ public static Set<FileObject> openProjectFiles (Project p) {
541526

542527
public static Set<FileObject> openProjectFiles (Project p, Group grp) {
543528
String groupName = grp == null ? null : grp.getName();
544-
ERR.log(Level.FINE, "Trying to open files from {0}...", p);
529+
LOG.log(Level.FINE, "Trying to open files from {0}...", p);
545530

546531
List<String> urls = getOpenFilesUrls(p, groupName);
547532
Set<FileObject> toRet = new LinkedHashSet<>();
548533
for (String url : urls) {
549-
ERR.log(Level.FINE, "Will try to open {0}", url);
534+
LOG.log(Level.FINE, "Will try to open {0}", url);
550535
FileObject fo;
551536
try {
552537
fo = URLMapper.findFileObject (new URL (url));
@@ -555,13 +540,13 @@ public static Set<FileObject> openProjectFiles (Project p, Group grp) {
555540
continue;
556541
}
557542
if (fo == null || !fo.isValid()) { //check for validity because of issue #238488
558-
ERR.log(Level.FINE, "Could not find {0}", url);
543+
LOG.log(Level.FINE, "Could not find {0}", url);
559544
continue;
560545
}
561546

562547
//#109676
563548
if (ProjectConvertors.getNonConvertorOwner(fo) != p) {
564-
ERR.log(Level.FINE, "File {0} doesn''t belong to project at {1}", new Object[] {url, p.getProjectDirectory().getPath()});
549+
LOG.log(Level.FINE, "File {0} doesn''t belong to project at {1}", new Object[] {url, p.getProjectDirectory().getPath()});
565550
continue;
566551
}
567552

‎ide/projectui/src/org/netbeans/modules/project/ui/groups/Group.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,19 @@ static void open(final Group g, String oldGroupName, boolean isNewGroup, Prefere
562562
h.start(200);
563563
ProjectUtilities.WaitCursor.show();
564564
final OpenProjectList opl = OpenProjectList.getDefault();
565-
Set<Project> oldOpen = new HashSet<Project>(Arrays.asList(opl.getOpenProjects()));
565+
566+
Set<Project> oldOpen = new HashSet<>();
567+
for (Project open : opl.getOpenProjects()) {
568+
// TODO fix this properly, e.g investigate if:
569+
// - getOpenProjects() should only return unboxed projects
570+
// risk: called by public API
571+
// - review/fix the broken hashcode/equals contracts
572+
// risk: code contains hacks which account for this already, e.g if (a.equals(b) || b.equals(a))
573+
// for now: unbox potential fod.FeatureNonProject wrapper since it breaks Sets/Maps due to incompatible hashcode/equals impls
574+
Project real = open.getLookup().lookup(Project.class);
575+
oldOpen.add(real != null ? real : open);
576+
}
577+
566578
//TODO switching to no group always clears the opened project list.
567579
Set<Project> newOpen = g != null ? g.getProjects(h, 10, 100) : getProjectsByPreferences(noneGroupPref, h, 10, 100);
568580
final Set<Project> toClose = new HashSet<Project>(oldOpen);

0 commit comments

Comments
 (0)