Che: Consistent Lifecycle Events for Projects

[tsmaeder]

I have been analysing selenium failures in the jdt.ls branch, and one of the reasons stuff fails is that we rely on Project created events. The idea is to make jdt.ls become aware of all projects that che is aware of.
Looking at the Events in org.eclipse.che.api.project.server.notification, I have noticed that they are not sent from all code paths creating projects.
For example, ProjectCreatedEvent is sent from ProjectServiceAPI#createProject(), but when going through ProjectServiceAPI#importProject(), no such event is sent.
I see no reliable way to implement the needed functionality for jdt.ls with the current lifecycle event story. If I am wrong on this count, I would be grateful for guidance on how to do it.
In general, I think our code would benefit from making lifecycle events more consistent. As I read the code, org.eclipse.che.api.project.server.impl.ProjectConfigRegistry holds the ultimate truth of whether a project exists in Che, so I think events should be sent from there.

[gazarenkov]

I believe the best place for adding it is ProjectManager (I believe it should act as a facade of all project related operations), let's keep ProjectConfigRegistry as a registry only not adding additional responsibility on it.

[tsmaeder]

Maybe it makes sense to make the interface IProjectConfigRegistry package private since it's not really supposed to be implemented by someone else.

[dkuleshov]

I think it's a reasonable suggestion by @tsmaeder to make project related events consistent. I agree with @gazarenkov that we should keep project registry as simple as possible, it has it's own humble responsibilities.

I'm also concerned that ProjectManager and current project lifecycle are not quite ready to support consistent project related events, I mean if we're talking about real consistency then it is not about just addition of events in corresponding places, it is a more challenge than it may seem at first. Most obvious problem - we yet don't have clear vision on how we should treat (in context of events) projects created outside of our API (vis SSH). In actual implementation we support creation of a project by simply running mkdir /projects/che in a terminal (project API is not engaged).

We catch a corresponding FS event then we rise a corresponding project lifecycle event. And here I'm asking myself several questions:

• How will I distinguish FS event that caused by project API and the one that caused by outer sources? FS event don't care if a directory was created via SSH or project API, so I need to exclude by myself FS events in such cases or I will have duplicated events (from ProjectManager and from FS).
• If I rely only on FS events how can I know when a project creation is finished not to rise an event too early, how I distinguish project removal and project renaming, etc.?

So before starting the work we need to clarify if we either need to develop an understanding of how we will further handle project life cycle events along with file system events, or we sacrifice full consistency for the sake of a simpler solution, which will be enough for what @tsmaeder initially asks.

One more important thing, when it is said:

I see no reliable way to implement the needed functionality for jdt.ls with the current lifecycle event story.

Is it meant that eventually we will need to have a way to transfer project lifecycle events into parallel container that JDT language server will be run in or is it meant that we are not initially aimed on running the language server in a parallel container?

[gazarenkov]

Well, ProjectConfigRegistry is already an interface with 2 implementations (second one - experimental)... do not mind to make it package private, indeed, it is not something to be implemented externally.

[tsmaeder]

@dkuleshov I don't see the problem quite as difficult as you do. "Existence" as far as Che is concerned means that there is a ProjectConfig for the project. When Che first becomes aware of a workspace folder (be it via project API or via FS events, it should send out a "created" event, when it receives a notification for something that already exists, it should send a "update" event. We could be smart and filter out updates that introduce no changes in the ProjectConfig. Am I missing something?

[dkuleshov]

@tsmaeder I'm always seeing it all more difficult :D I'll try to explain my concerns more. If you take a look here org.eclipse.che.api.project.server.impl.ProjectServiceApi#createProject and go deeper you will see that currently we rise a ProjectCreatedEvent not on the creation of a directory but right after a project and all its content is created (source files, resources, etc.), if I remember we do the same when we import a project from a GitHub. So I guess throwing a ProjectCreatedEvent while project import/creation is still in progress is an unexpected by design behavior.

I'm thinking on introducing separate events for logical (project created/updated) and physical (directory/file created) events, will it be useful for you case, what do you think?

[gazarenkov]

I'd say it is 2 independent events: like folderCreated(path) and projectCreated(project metainfo). generally speaking they can be emitted by different machines (project service can be remote from project fs)
and, they can be transmitted to other remote workspace machines, I think?

[gazarenkov]

@tsmaeder I believe folder creation is not that important for jdt.ls.
I think, if you want to launch it in advance (not on java file opening), it can rely on project created/updated event when ProjectType.isTypeOf(Java) (which including maven etc)?

[tsmaeder]

@dkuleshov I think that the addition of content can be done before or after creating the project, that doesn't really matter. We may always add more content after creating the project, so we need to handle the case anyway. We may have to use some kind of mutual exclusion ("lock workspace") to ensure we don't get an early creation event via the file system events in cases where we know that we are adding content right away. Eclipse has locking mechanisms for resource scopes exactly for this purpose, I think the same approach applies.
@gazarenkov I really think it would be better to leave the decision whether jdt.ls is interested in a project up to jdt.ls. There are generally only few projects (maybe hundreds max), so notifying about creation of them should not be all that expensive.

[tsmaeder]

@dkuleshov what I mean is, that instead of doing this:

projectManager.createProject()
put project contents on disk...

We could do

lock the workspace
put project contents on disk
projectManager.createProject();
unlock workspace.

[gazarenkov]

@tsmaeder ok, good to know it is just general thoughts, not related to jdt.ls

So, as discussed, I think, we can consider making eventing more consistent generating projectCreated event on ProjectManager's import() operations.

[tsmaeder]

As far as jdt.ls is concerned, I'm interested in getting a notification whenever a new project config appears. But generally, I think handling the lifecycle of RegisteredProject instances in a single and sendint events from there class will make the code more robust: no need to think about it when adding new code.

[vparfonov]

@tsmaeder So for my vision you need to get Event when a new folder is created in ROOT of workspace and not depends when source will be imported. I am right?

[gazarenkov]

@vparfonov I believe the interesting event is Project Created or Updated, it should be called in all cases we create a project with explicitly assigned configuration (its including project import), even if it is Default (Blank) project type.
In case of just creating folder in the projects root - configuration is not assigned explicitly => Project even is not fired.
I do believe it is a right assumption which should work correct for all the cases including LS initialization.

[dkuleshov]

@tsmaeder

We could do

lock the workspace
put project contents on disk
projectManager.createProject();
unlock workspace.

According to current specification workspace file system can be accessed both with our API and without it (e.g. SSH). There is no efficient way how we can lock workspace for outside (non-API) operations. Turns out that we can't do what you assumed we could.

So we either have a challenging task to consider project events in context of both in- and outside API operations to make project lifecycle events really consistent or we add more simple workarounds just for your specific use-cases and drop the idea of consistent lifecycle events.

In my opinion both options are viable, we need to take the decision, update the description and start the work.

@gazarenkov

@vparfonov I believe the interesting event is Project Created or Updated, it should be called in all cases we create a project with explicitly assigned configuration (its including project import), even if it is Default (Blank) project type.
In case of just creating folder in the projects root - configuration is not assigned explicitly => Project even is not fired.
I do believe it is a right assumption which should work correct for all the cases including LS initialization.

If I understood correctly you describe current behavior. When we create/update a project through project API then an event is fired. When we create just a directory in the root of the workspace through API (e.g. zip import) or via SSH then an event is not fired. So if it is correct and expected behavior then there is nothing to do or discuss more here.

[gazarenkov]

@dkuleshov as I understand we do not fire projectCreated event on project import (in particular) which does not look consistent (maybe there are other cases like this, not sure)
So, my understanding is that we need to fix it?

[dkuleshov]

@gazarenkov seems that it's a bug, if that would be enough then okay.

[tsmaeder]

As far as jdt.ls is concerned, I'm interested in getting a notification whenever a new project config appears. But generally, I think handling the lifecycle of RegisteredProject instances in a single and sendint events from there class will make the code more robust: no need to think about it when adding new code.

[tsmaeder]

I was blocked on my other task yesterday and did some hacking...I could probably propose a patch in the next 1-2 days.

[vparfonov]

We will propose PR today and launch CI test

[gazarenkov]

Well, I would not qualify it as a bug, but OK, if you, guys, prefer so.
I'd just like to rename the issue making the title more clear about the problem we solve, like "Fire ProjectCreated event everywhere we create it explicitly (including Project import)".
It will make Changeslog more understandable.