[Asterisk-code-review] Fixed duplicated subscription adding (...asterisk[master])

[sungtae kim]

sungtae kim has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/11201 )

Change subject: Fixed duplicated subscription adding
......................................................................


Patch Set 2:

> Patch Set 2: Code-Review-1
> 
> After thinking this through more and looking at the code I don't think this is the correct solution. The only case where you can have a subscription subscribed multiple times is actually using the forwarding code. You can't directly do it. The intention of the forwarding code is to ensure that messages for one topic are forwarded to the subscribers of another. The behavior right now results in multiple messages, which is not ideal. I'm worried however that with this change once the first forward is canceled the subsequent ones are no longer in effect, so messages won't be forwarded. This goes against the intention of the forwarding.
> 
> I think there's two options:
> 1. As long as 1 forwarding is still active the forwarding still occurs. It is only when the last one is canceled that it stops.
> 2. Trying to set up forwarding again involving the same topics results in an error that they are already forwarded.
> 
> I'm not certain which is the best option really. On one hand with option 1 it would just work. On the other hand trying to forward between the same topics again seems like a bug.

Hi Joshua,

Thank you for your kind review.

Yes, I can see your point. I agree with you.

I'm not feeling good for option 1. Imho, it's not the correct way to fixing it. It looks like hiding the bug. It will come back as a bigger problem in the future.

So, I'd choose the 2nd one. Here's some code fix for option 2.

I just made a patch for that without your confirming. But just feel free to let me know if you think option 1 is better. :)


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11201
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I00bd627e4cc21aba0535d2695460a04f06f6e699
Gerrit-Change-Number: 11201
Gerrit-PatchSet: 2
Gerrit-Owner: sungtae kim <pchero21 at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: sungtae kim <pchero21 at gmail.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:16:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190408/1971cea6/attachment.html>