ee/integration #183
ee/integration #183
Conversation
…e/transition-unit-tests
…e/transition-unit-tests
…e/transition-unit-tests
|
wrong target |
mohitpubnub
changed the base branch from
master
to
eventengine/transition-unit-tests
| subscribeOperation.SubscribeListenerList = subscribeCallbackListenerList; | ||
| //subscribeOperation.CurrentPubnubInstance(this); |
There was a problem hiding this comment.
@budgetpreneur Do you think we will need this?
| var emitMessageHandler = new Effects.EmitMessagesHandler(pubnubInstance, messageListener); | ||
| dispatcher.Register<Invocations.EmitMessagesInvocation, Effects.EmitMessagesHandler>(emitMessageHandler); | ||
|
|
||
| var emitStatusHandler = new Effects.EmitStatusEffectHandler(pubnubInstance, statusListener); |
There was a problem hiding this comment.
Do we need link statusListener and messageListener with pubnubInstance ? Are we not instantiating SubscribeEventEngine per Pubnub instance.
There was a problem hiding this comment.
Pubnub reference was needed as all listener delegates signature are : (Pubnub instance, ...). So while calling those delegates along with data effect handler got, It has to pass pubnub instance reference along with it.
Other then that no other relation handler has with pubnub instance.
There was a problem hiding this comment.
I'm referring this line
src/Api/PubnubApi/Pubnub.cs
Outdated
| else { | ||
| var subscribeManager = new SubscribeManager2(pubnubConfig[InstanceId], JsonPluggableLibrary, pubnubUnitTest, pubnubLog, telemetryManager, tokenManager, this); | ||
| if (subscribeCallbackListenerList!= null && subscribeCallbackListenerList.Count > 0) { | ||
| messageListener = subscribeCallbackListenerList[0].Message; |
There was a problem hiding this comment.
subscribeCallbackListenerList[0].Message
Any reason for looking at only one listener?
src/Api/PubnubApi/Pubnub.cs
Outdated
| var subscribeManager = new SubscribeManager2(pubnubConfig[InstanceId], JsonPluggableLibrary, pubnubUnitTest, pubnubLog, telemetryManager, tokenManager, this); | ||
| if (subscribeCallbackListenerList!= null && subscribeCallbackListenerList.Count > 0) { | ||
| messageListener = subscribeCallbackListenerList[0].Message; | ||
| statusListener = subscribeCallbackListenerList[0].Status; |
There was a problem hiding this comment.
subscribeCallbackListenerList[0].Status
Any reason for looking at only one listener?
src/Api/PubnubApi/Pubnub.cs
Outdated
| savedSubscribeOperation = subscribeOperation; | ||
|
|
||
|
|
||
| subscribeEventEngine.eventQueue.Enqueue(new SubscriptionChangedEvent() { Channels = this.GetSubscribedChannels().ToArray(), ChannelGroups = this.GetSubscribedChannelGroups().ToArray() }); |
There was a problem hiding this comment.
GetSubscribedChannels() and GetSubscribedChannelGroups() will be null at the first time.
Even I think, this is not the place in Pubnub.cs.
Can we move this code block inside SubscribeOperation2.cs (better name would be better)?
There was a problem hiding this comment.
IMHO, Pubnub.cs is facade layer for all the operations that we want to expose to end users.
There was a problem hiding this comment.
WIP: finding more cleaner way!!!
There was a problem hiding this comment.
@budgetpreneur If I expose getSubscribedChannels() in SubscribeOperation2 then It will be easy to get that info for eventEngine.
Because otherwise we need to pass something related to event engine into SubscribeOperation2.
There was a problem hiding this comment.
@budgetpreneur please check last commit
mohitpubnub
force-pushed
the
ee/integration
branch
from
929ed27 to
5741c2d
Compare
|
another PR is in place #185 |
emitMessageandemitStatus