cirular dependency for iWF client
longquanzheng opened this issue · 4 comments
iwfClient depends on Registry (so that it knows the workflow types, input types, RPC types etc)
Registry depends on all the workflows that need to be reigstered
Now, if there is a workflow need to use iwfClient, it becomes a circular dependency (e.g. if a workflow want to use iwfClient to start/signal another workflow)
Potential fix:
Let registry
to be "Lazy" for iwfClient so that it don't pass in to create iwfClient. And it will set the registry later after bean created. Basically move the "lazy" annotation to internal and hide it from users
One solution can be: In Registry, add a method configClient to set the registry for the client, so when creating beans, Registry doesn't need to be a parameter passed to the Client bean.
The related code changes are:
public class Registry {
public void configClient(final Client client) {
client.setRegistry(this);
}
}
public class Client {
private Registry registry; // remove the final annotation
public Client(final ClientOptions clientOptions) {
this.clientOptions = clientOptions;
this.unregisteredClient = new UnregisteredClient(clientOptions);
}
public void setRegistry(Registry registry) {
this.registry = registry;
}
}
So when creating beans:
@Bean
public Registry registry(Client client, ObjectWorkflow... workflows) {
Registry registry = new Registry();
Arrays.stream(workflows).forEach(registry::addWorkflow);
registry.configClient(client);
return registry;
}
@Bean
public Client client(
final @Value("${iwf.worker.url}") String workerUrl,
final @Value("${iwf.server.url}") String serverUrl) {
return new Client(
ClientOptions.builder()
.workerUrl(workerUrl)
.serverUrl(serverUrl)
.objectEncoder(new JacksonJsonObjectEncoder())
.build()
);
}
And should we deprecate the public Client(final Registry registry, final ClientOptions clientOptions)
constructor to avoid confusion?
@longquanzheng What do you think?
@zklgame Yes, the high-level idea sounds great. And thanks for providing the details.
I just want to comment more on the details:
- Maybe we could have a better way than a public constructor for Client, so that it's clear that the client is "half-way created" after the API call. And the instance needs to be passed to a registry for further initialization before being used
- If for some reasons, the Client instance is being used before further initialized by registry, can we throw some clear exceptions, if the APIs require registry (e.g. start/signal WF, I think at least half of them require registry).
resolved in indeedeng/iwf-java-samples#16
turned out we made a wrong use of Registry in Spring