wilkinsona / project-generator

3 stars 4 forks source link

Project generation event not fired #80

Closed mbhave closed 5 years ago

mbhave commented 5 years ago

The old API triggers a ProjectGeneratedEvent after project generation. ProjectGenerationMetricsListener listens to this event.

snicoll commented 5 years ago

For the record, I didn't bother pushing that down to the "invocation layer" as we're not sure yet what exception type we're going to use and I think that's a big part of this work.

The issue title is very generic and you haven't shared a failing test so I don't really know what you mean but I expect a project request from the outside (i.e. request from the web ui) to fire the event.

mbhave commented 5 years ago

I am looking at the ProjectGenerationTests to determine if we have test coverage for them elsewhere so that they along with ProjectRequest can go away. Some tests in there expect the event to be fired so unless we have that somewhere else, those tests can't be removed I think.

snicoll commented 5 years ago

Event handling is not managed yet.

It is tricky because triggering a failure boils down to several cases that can only happen concretely when parsing ProjectRequest as you can't really put an invalid value (unless you don't specify one at all) with ProjectDescription:

There is an extra exception if we attempt to extra the build system from the type and it does not exist (i.e. foo-project).

All the above should happen upfront. There is one case where that exception can be throw by the new API: if you request to generate a Maven build and you've configured the request to be a Gradle build. But, again, we can check that upfront.

All in all, I have a feeling that invalid requests might be a web specific thing. We still need to validate basic stuff like "the build system is provided" but that looks more pilot error using the API.

Interestingly enough, triggering an event a project was successfully generated belongs probably to the new API so I guess we need a base event in the new API that the "web layer" can extend from for invalid requests.

snicoll commented 5 years ago

More fun. We need to pass down some generic context as the listener will check if some HTTP headers are set.

snicoll commented 5 years ago

There is also a case where we check the raw dependencies identifiers for dependencies without metadata.

snicoll commented 5 years ago

We've decided to keep event handling in the invocation layer for the time being. We'll have to handle this at some point so I'll create a follow-up issue on this work is merged.