Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add CdiEventListener to processApplication by default #122

Closed

Conversation

jangalinski
Copy link
Contributor

changes:

  • add transitive dependency to engine-cdi (must be deployed together with the ejb-client.jar so no use in scoping it "provided"
  • include the CdiEventListener setup from the web site example

Effect: using the default ejb processApplication enables cdi eventing by default without the need to write and deploy a custom PA.
Figuring out the correct usage took some time and I dont want to write a custom processApplication for every JEE/BPM project.

discussion:

  • EJB is not CDI so one could argue that this is breaking, since I could run on a container not supporting CDI. But is pure JEE5 a supported platform for 7.2.0?
  • Performance: a view events get fired even if no one is listening. Idea: enable/disable via configuration. How about checking if the ProcessApplicationEventListenerPlugin is enabled? How can I access the processEngineConfiguration from within the ProcessApplication to check?

changes:
- add transitive dependency to engine-cdi (must be deployed together with the ejb-client.jar so no use in scoping it "provided"
- include the CdiEventListener setup from the web site example

Effect: using the default ejb processApplication enables cdi eventing by default without the need to write and deploy a custom PA.
Figuring out the correct usage took some time and I dont want to write a custom processApplication for every JEE/BPM project.

discussion:

- EJB is not CDI so one could argue that this is breaking, since I could run on a container not supporting CDI. But is pure JEE5 a supported platform for 7.2.0?
- Performance: a view events get fired even if no one is listening.  Idea: enable/disable via configuration. How about checking if the ProcessApplicationEventListenerPlugin is enabled? How can I access the processEngineConfiguration from within the ProcessApplication to check?
@buildhive
Copy link

camunda BPM » camunda-bpm-platform #987 UNSTABLE
Looks like there's a problem with this pull request
(what's this?)

@jangalinski
Copy link
Contributor Author

I know that tests are failing. I wanted to use this PR to start a discussion if this is something you want or can go with. I won't go on and fix all integration problems (mainly because engine-cdi is missing) until we get an "OK, great idea!" from the core team ... otherwise, I could imagine a community extension ProcessApplication that is not limited by some not-up-to-date AS.

@meyerdan
Copy link
Member

Hi Jan,

I see the same downsides as you.

EJB is not CDI so one could argue that this is breaking, since I could run on a container not supporting CDI.

Yes and even if the container does support CDI I may choose not to use it or I may choose not to use camunda-cdi.

I think that it is good to check our options for making the event support available to users more easily. But maybe we could think about a more elegant solution which does not have the downsides you named yourself.
Maybe it could be configurable through CDI using @Alternative or something?

Cheers,
Daniel

@meyerdan
Copy link
Member

How about checking if the ProcessApplicationEventListenerPlugin is enabled?

If the plugin is not enabled, there is no downside to adding the listeners since they will simply not be called.

@jangalinski
Copy link
Contributor Author

Yes. But is it possible to check from inside the process application if the plugin is enabled? Then I could

  public ExecutionLIstener getExecutionListener() {
        return isPluginEnabled() ? cdiEventListener : null;
  }

so we can ignore any @Alternative/@specializes handling. Just enable the plugin and add engine-cdi to the war ...

Question: what does "isPluginEnabled()" look like?

@meyerdan meyerdan self-assigned this Oct 7, 2015
@meyerdan
Copy link
Member

meyerdan commented Oct 7, 2015

Hi jan,

same question here. Are you still interested in completing this PR or should we close it?

Thanks,
Daniel

@jangalinski
Copy link
Contributor Author

I am currently not working on any EJB/CDI stuff, personal interest and customer projects shifted to spring (boot/tomcat), so I have no plans to continue right now. But my origininal question ("I won't go on and fix all integration problems (mainly because engine-cdi is missing) until we get an "OK, great idea!" from the core team") was never answered ... would you want this solution?

@meyerdan
Copy link
Member

meyerdan commented Oct 7, 2015

Hi Jan,

at the moment we do not have the possibility to look into it ourselves. So I'll close it.

I prefer that you work on spring boot etc ... 💨

Daniel

@meyerdan meyerdan closed this Oct 7, 2015
tmetzke pushed a commit that referenced this pull request Feb 13, 2020
* master:
  added configuration migration hint
  complete authorization configuration #118 as mentioned in #35
  added property for default serialization format closes #127
  added property for authorization enabling (defaults to true). #118
  add documentation for camunda-bpm-spring-boot-starter-rest closes #110
  changed configuration docs to meet #86
  to get a first feeling for ProcessEnginePlugin as configuration replacement #56
  introduce properties for metrics (#124)
  use of constants for commit 9198b67
  added dependencies for webapp-classes (required in 7.6)
  removed usage of internal implementation classes (one at least changes with 7.6)
  add metrics configuration to context, fixes #122
  add enabled comment, fixes #46
  introduce properties for metrics (#124)
  add Roadmap
  separate compile for extensions and examples
  typo
  to get a first feeling for ProcessEnginePlugin as configuration replacement #56
  typo
  to get a first feeling for ProcessEnginePlugin as configuration replacement #56

# Conflicts:
#	README.md
#	examples/pom.xml
#	extension/starter/src/main/java/org/camunda/bpm/spring/boot/starter/configuration/CamundaConfiguration.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants