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

implement artifactFactory for spring using applicationContext #179

Closed

Conversation

jangalinski
Copy link
Contributor

I implemented an implementation of ArtifactFactory for Spring utilizing a applicationContextAware engine Configuration.

@martinschimak
Copy link
Contributor

👍 And good to know that you did it already before I started to do it.

@buildhive
Copy link

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

@bconnection-zz
Copy link

Hello Jan,
that looks fine. I supplemented the following:

    public <T> T create (final Class<T> springManagedClass, final Object ...args){
        return this.context.getBean(springManagedClass, args);
    }

Then it is also possible to use parameters at constructors.

Thank you for your work.

Matthias

@jangalinski
Copy link
Contributor Author

Hi Matthias I do not understand what you mean ... the signature defined by the ArtifactFactory interface is fixed ... how would you use your additional code?

@bconnection-zz
Copy link

Hello Jan,
with your code you can write things like:

@Autowired
SpringArtifactFactory factory;

void someMethod(){
    SomeClassType instance = factory.createArtifact(someClass);

With my extension you could additional write:

   SomeClassType instance = factory.createArtifact(someClass, param1, param2);

With the second variant you are able to utilize also constructors which require arguments.

Sorry, I did not apply to the identifiers you introduced in your code, instead I copied the example from my implementation.
Kind regards
Matthias

@jangalinski
Copy link
Contributor Author

I do get what the code might be good for. But in this special case, we want to use a standard interface of the camunda platform, mplement it for spring so that when a listener or delegate has to be created for a task while executing a process, we can provide a fqn of the class and we will get not the standard constructor instance but a spring bean with injection. Your extension on the other hand is a wrapper for the applicationContext, I would not need the artifactFactory at all, I could just operate on the context directly. Or am I missing something?

@bconnection-zz
Copy link

Yes, I did the implementation, because I was not aware of the possibilities in Camunda. I posted my proposal, because I couldn't find any hints, how I could instantiate Spring classes.

I read the class string from the bpmn KeyForm attribute within a execution listener. Therefore it is my code to instantiate the classes. So the second form became useful for me.

The most tricky part was: how could I provide access to the application context, because it requires to implement the ApplicationContextAware Interface. Spring will provide the context by wiring it at startup. Thus the required scope for the factory is "singleton". But if the scope is "prototype" like the classes you will instantiate during execution you need the factory. The factory is singleton and knows the context and can create instances of arbitrary objects during execution time of the system.

To introduce it in the expressions, the parameterless form is sufficient.

@jangalinski
Copy link
Contributor Author

"To introduce it in the expressions, the parameterless form is sufficient. " .... but we do agree, that (at least with the code base today), no other than the parameterless form is supported by either the artifactFactory interface or the camunda:extension for bpmn. So this might be a good idea in the future, but is not applicable for this PR, right?

@bconnection-zz
Copy link

Yes, for the use via Camunda.

But if you autowire the SpringArtifactFactory you may also use the parameter based variant.

@saig0
Copy link
Member

saig0 commented Sep 29, 2015

Hi Jan,

thank you for your Pull Request. It looks good!

But before I merge it, can you please provide more unit tests for process execution (start a process instance which uses the SpringArtifactFactory for resolving the class of a service task or task listener - see ServiceTaskSpringDelegationTest for example). Additionally, please use xml configuration instead of annotations like the other test cases.

Greetings,
Philipp

@saig0
Copy link
Member

saig0 commented Oct 14, 2015

@jangalinski: Please provide the requested changes. Otherwise, I have to close the pull request for now. Note that you can re-open it later again.

@jangalinski
Copy link
Contributor Author

Sorry, been busy. Will provide tonight

@saig0
Copy link
Member

saig0 commented Oct 14, 2015

perfect 👍 thank you.

@jangalinski
Copy link
Contributor Author

I am having trouble with my setup that I wont be able to fix before the weekend. So unless someone wants to take over and provide the xml-context tests, this has to wait, sorry.

@saig0
Copy link
Member

saig0 commented Oct 15, 2015

So we wait till next week. No problem :)

@saig0
Copy link
Member

saig0 commented Nov 4, 2015

@jangalinski: Next release comming at the end of this month. You should provide the changes soon. Otherwise, this feature is not a part of the current release.

@saig0
Copy link
Member

saig0 commented Nov 9, 2015

@jangalinski : I will close this PL for now. You can re-open it when you have time to complete the changes. If you complete it till wednesday I will merge it for current release.

@saig0 saig0 closed this Nov 9, 2015
@jangalinski
Copy link
Contributor Author

Yes, good idea ... thanks for your patience.

koevskinikola pushed a commit that referenced this pull request Aug 12, 2020
tasso94 pushed a commit that referenced this pull request Nov 10, 2020
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.

5 participants