ProjectManager.findProjectOrFallback API

[jtulach]

• introducing ProjectManager.findProjectOrFallback to allow viewing every folder as a generic fallback project
• this is an API review extracted from PoC: More prominent Open Folder (as a group) action #9631 proof of concept work
  • I intend to run the review as API fast track review
  • e.g. accept comments and if there are no objections integrate in a week - e.g. on Oct 8th
• this PR contains the necessary API change, tests covering its behavior, its documentation, proper versioning and dependencies among projects
• the PR also contains two "obvious" usages
  • when opening after start
  • when creating Folder of Projects group
• fix MultiSourceFileProvider - done in c22ea60
• NPE or not NPE? That's the question!

[jtulach]

One obvious usage of this new "fallback project" functionality is opening of projects on startup.

The files on disk may change since the NetBeans shutdown. Sometimes it happens that original projects are no longer recognized even the original directory path exists.

In such case it is still better to open the "fallback project" rather than opening nothing.

[jtulach]

When using File/Project Groups... menu:

Selecting the menu item opens a dialog

and allows one to click the New group... button to open another dialog. After choosing Folder of Projects and browsing for it:

... one can click "Create Group" button. Which scans for all the projects in the given folder and opens them in Projects view (after closing all previous ones).

No Projects Found

It may happen that there are no projects found during the scan. Leaving the Projects View empty. That's very weird.

With this one line change, the root of the folder is always opened: either as a real project or as a "fallback project".

Works great with #9602 and its nesting capabilities. The root folder is opened on the top and all other projects are nested beneath it.

[neilcsmith-net]

This exception message seems off? Doesn't this throw even if the fallback isn't needed?

Personally I'd prefer a default method over the additional type here, but I know your feelings there.

[jtulach]

Done in c01de98 - together with introduction of a new argument type, it is binary compatible change as analyzed here.

[neilcsmith-net]

Thanks! Almost what I was going to suggest with the opening flags. I don't agree with all of your post on default methods (JSONList), but I do understand the need for considering compatibility.

In light of which, I am still concerned about the exception in ProjectManager, and the potential for certain places to start throwing exceptions that didn't before. I'd still allow findProjectOrFallback to return null if we're going to use it as a replacement.

[jtulach]

concerned about the exception in ProjectManager

Which exception do you have in mind? The NPE that could be throw here? Well, that's never going to be thrown as with FindOptions.WITH_FALLBACK argument the implementation never returns null. Just (as a result of introducing a default method for all the FindOptions variants) the type system (nor @NonNull & co. annotations) really can express that easily.

[neilcsmith-net]

Which exception do you have in mind? The NPE that could be throw here? Well, that's never going to be thrown as with FindOptions.WITH_FALLBACK argument the implementation never returns null.

Exactly that NPE. Sure, your implementation never returns null, but the public SPI interface default method does. Now, if we're treating that as an SPI that can only have one implementation, that's fine, but then the arguments about maintaining compatibility don't really hold.

So, we could have ProjectManager::findProjectOrFallback throw an exception like this in cases where the implementation doesn't override the default method. But it is not then a simple drop-in replacement for ProjectManager::findProject. Your changes in DirectoryGroup and OpenProjectList retain the null check that is no longer needed, but will hit a different code path if an SPI implementation causes an exception - the latter will catch it, but the former will blow up I think?

So, if we want to treat this as a simple drop-in replacement across our internal code and treat it as a public SPI, I think it's easier to allow a null return as with ProjectManager::findProject when fallback creation fails. If not, then the usages in this PR might need a rethink.

[jtulach]

Exactly that NPE.

OK. I'll think about our alternatives.

if we're treating that as an SPI that can only have one implementation

• I believe we do
  • the other implementation was part of Oracle's Project TailWind which is long time dead as far as I can imagine, CCing @tzezula and @sdedic
• e.g. for all practical purposes there is just a single implementation NbProjectManager
• the assumption there was just a single implementation was the major reason why usage of default methods seemed justifiable in this case to me

[lahodaj]

(Whatever we do in this area, @eppleton is right we need a more proper check in the source file launcher check; that can be done separately.)

I assume the end goal here is to achieve the "Open Folder"/"Add to Workspace" actions, as mentioned in the recent e-mail. I do wholeheartedly agree with adding (some) support for "workspaces" (i.e. ability to work without projects, or without specifying projects one-by-one).

What I am not quite sure if whether starting with an API change is the best move. Whether mapping this to project groups matches the requirements also remains to be seen.

I think I would first try to implement the actions, and from that it would lead what API changes are needed (I suspect some will be needed). #9631 implemented the Open Folder as Workspace action, but not the Add to Workspace action; and also the resulting UI is I think not ideal: when I do "Open Folder as Workspace", would I really expect to see all the nested projects in the Projects tab? (I realize this is a consequence of reusing the Project Groups directly, but a novice user is likely to be confused by this, I think.) OTOH, I do agree it would be good to open the projects under the workspace folder.

I guess if I would be doing this, I would:

• try to make the feature end-to-end first. It does not have to be merged as a single PR, but ensuring the end result is as good as possible before adjusting APIs would be nice
• if there were explicit actions to open a folder as a workspace/add folder to a workspace, we wouldn't probably need the fallback API - the "workspace folder project" could be created by an ordinary project factory, if needed; (the action would mark the directory as "needs workspace project, if there's no better" in the userdir, or something alike)
• some part of the system would, when a project matching the workspace folder directory opens, scan the given folder and open all nested projects
• there could be a UI tweak to not show these projects, unless explicitly open by the user; this is probably the trickiest part to implement, and would probably need an API

[MartinBalin]

Fallbacks break existing project recognition consumers
Once cached, a fallback is returned by ordinary findProject(). Consequently, FileOwnerQuery assigns previously unowned Java files to it, disabling single-file Run/Debug and classpath support, which require a null owner. It also makes ProjectGenerator.createProject() reject the folder as already containing a project. Preserve these existing consumers’ behavior before exposing fallbacks through the ordinary cache.

Cache clearing returns before replacing fallbacks under write access — NbProjectManager.java (line 517)
postReadRequest() defers execution until the current write lock is released. Creating project metadata, calling clearNonProjectCache(), then calling findProject() within that lock therefore still returns the fallback. Replacement needs to take effect before the subsequent lookup.

Replaced fallbacks remain valid and can retain file ownership — NbProjectManager.java (line 525)
Removal updates only dir2Proj. The fallback remains in proj2Factory, so isValid(fallback) stays true. When replacement is triggered by a factory lookup change, the file-owner cache also remains stale: findProject() returns the real project while FileOwnerQuery returns the fallback. Retire the old instance and invalidate ownership caches together.

[jtulach]

Thanks Martine. Some of these are similar to Toni's findings. Some are new. All fixed in e348182

[MartinBalin]

A nested fallback’s source group can exclude its own root
Query ownership of a child folder inside an existing project, then create a fallback for that child. Ownership remains cached against the parent, so GenericSources.group(...).contains(root) returns false, violating the SourceGroup contract. Fallback registration must reconcile cached ownership with its source-group implementation.

[jtulach]

This one was tricky! A test based on your example and a fix is in as e348182. Thank you, Martine!

[jtulach]

Thank you Jan.

... the end goal here is to ...

• The end goal of this PR is to implement ProjectManager.findProjectOrFallback API change and use it at two places, where it makes obvious sense:
  • when opening after start
  • when creating Folder of Projects group
• please find my reply to your suggestion to provide new ProjectFactory in the newly started discussion thread associated with NbProjectManager class

What I am not quite sure if whether starting with an API change is the best move.

• the goal of this PR is to avoid changing the UI (and that's a good move from my perspective)
  • if you have concerns about deeper changes to UX, I suggest you to express them in PR-9631
  • please find my reply there
• leave this PR fully focused on the ProjectManager.findProjectOrFallback API change, if possible, thank you!

[lahodaj]

Thank you Jan.

... the end goal here is to ...

* The end goal of this PR is to implement `ProjectManager.findProjectOrFallback` API change and use it at two places, where it makes _obvious_ sense:

Ok, if the end goal is to introduce the API (and the two uses of the API), and nothing more, OK. Not sure if the API pulls its own weight in that case, but I'll leave that to you.

Given this API (should) pull(s) its own weight without referring to the "workspace UI" stuff, I assume that under the "workspace UI" approach, we will feel free to use or not use this API, as we see fit. I.e. we won't feel under pressure to use this API just because it exists. If it will be useful - great. If not, we simply won't use it.

[jtulach]

• Thanks for the reviews this PR has obtained so far.
• As far as I can tell, all the issues were addressed.
• Reviewers, please take another look. Thank you.

[eppleton]

This enables single-file support under a fallback, but MultiSourceRootProvider (around line 510, RootPathResourceImplementation.includes) still filters the source path with FileOwnerQuery.getOwner(fo) == null. So a loose file under a fallback now gets a source path, but that path excludes the file itself and its siblings.

I checked it in MultiSourceRootProviderTest on this branch: a folder with pack/A.java and pack/B.java, opened via findProjectOrFallback. findClassPath(A, SOURCE) returns a path rooted at the folder, but contains(A) and contains(B) are both false. Without the fallback, both are true.

Run/Debug is enabled now, but A can't use B and the files are probably not indexed. Maybe includes() should apply the same rule as here (no owner, or an owner without Java source groups), ideally through a shared helper so the two can't drift apart again.

[jtulach]

• Shared utility introduced in c22ea60
• but I don't understand the test case
• I tried pack.A referencing pack.B

• however that doesn't work in any previous version of NetBeans either
• maybe the whole java/java.file.launcher module is a bit fragile?

[dukehoff]

You need to activate Source File Launcher Indexing for it to work:

[JaroslavTulach]

• These options aren't even visible in non-Java projects
• need 86be29e to make them appear
• then the support work on JDK 25 in empty directories as well as non-Java projects

[neilcsmith-net]

What I am not quite sure if whether starting with an API change is the best move. Whether mapping this to project groups matches the requirements also remains to be seen.
...
I guess if I would be doing this, I would:

* try to make the feature end-to-end first. It does not have to be merged as a single PR, but ensuring the end result is as good as possible before adjusting APIs would be nice

* if there were explicit actions to open a folder as a workspace/add folder to a workspace, we wouldn't probably need the fallback API - the "workspace folder project" could be created by an ordinary project factory,

Sorry, having given this some more thought, I agree with @lahodaj here. There is more to discuss. I think the requirement for this new API is not currently proven, and handling this probably can, and should, be via an ordinary project factory. As we're effectively in our ramp down phase to the next release, the end user benefits for this are currently minimal, there may still be some fallout from the implementation, and the API may lock us in to something that is the wrong approach, I don't think we should fast track this in. Let's hold fire for the next cycle and consider along with the wider impact usage that goes with it.

So, that's a -1 from me to merging this prior to branching NB 32. Unless one of our active maintainers wants to make a strong argument in the other direction.

I would really like to see the ability to opt in to opening any folder as a project. Don't get me wrong, I'm not negative on solving the problem. It should be possible for the user to open any folder as a project. There is potential there to address a whole bunch of user requests, including but not just making the folder group function better. eg. I'd like to see the inverse of our current folder action Open Project of Folder that is Open Folder as Project. We could even see fallback project more as a basic project. Having it registered as a standard project factory allows for it to be configured or hidden as with anything else in the layer system. That might be important for platform users before we bake this facility into the API.

if needed; (the action would mark the directory as "needs workspace project, if there's no better" in the userdir, or something alike)

In terms of marker, I would suggest the presence of the default AuxiliaryConfiguration on a folder, by either file attributes or the shareable .netbeans.xml. The file attributes for these are imported on upgrade as of NB31, and either of those things are likely to be used for storing project information if we extend to that.