MINOR: Fix bug where StoreBuilder added after connecting store with processor

[bbejeck]

Fixes bug found in #5731 where we add a state store to the processor before adding the state store to the InternalTopologyBuilder

I've added a test which fails without this fix.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[bbejeck]

ping @guozhangwang @mjsax @vvcephei

[mjsax]

LGTM

[guozhangwang]

@bbejeck My understanding is that, generally:

1. storeBuilder is for adding the "embedded" store to a stateful processor, like the materialized store for aggregations an joins, and here for suppressions.
2. storeNames is used for connecting any additional stores to the underlying processors.

And inside addStateStore we will still call the function to connect the storeBuilder.name() with processorName, so we should never have a storeBuilder AND its storeName added into the storeNames anyways.

The unit test you added, is calling on processorNode which is an internal node class, hence users should never be able to call add

new StatefulProcessorNode<>("node", processorParameters, new String[]{"store"}, storeBuilder, false);

And as long as we make sure 1) and 2) are mutual exclusive I think this should be fine?

[bbejeck]

@guozhangwang

That is correct, as I think about it more as I stated in the slack channel, in practice we'll only get either store names or a store builder passed in. Should I just close this PR?

[guozhangwang]

Should I just close this PR?

Yeah, if you agree with my reasoning I think this is not an issue then.

[bbejeck]

Yes I do agree, closing this PR