Eyepea / aiosip

SIP support for AsyncIO (DEPRECATED)
Apache License 2.0
82 stars 41 forks source link

Update dialog tracking to use tags & call_id #112

Closed ovv closed 6 years ago

ovv commented 6 years ago

Added async-timeout in dev requirements as it's useful for testing & debugging

ovv commented 6 years ago

Fix #96

codecov-io commented 6 years ago

Codecov Report

Merging #112 into master will decrease coverage by 2.14%. The diff coverage is 71.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   73.62%   71.48%   -2.15%     
==========================================
  Files          15       15              
  Lines        1547     1487      -60     
  Branches      267      256      -11     
==========================================
- Hits         1139     1063      -76     
- Misses        298      316      +18     
+ Partials      110      108       -2
Impacted Files Coverage Δ
aiosip/transaction.py 65.48% <ø> (-0.73%) :arrow_down:
aiosip/protocol.py 59.22% <33.33%> (-3.16%) :arrow_down:
aiosip/dialog.py 75.42% <77.77%> (-2.47%) :arrow_down:
aiosip/peers.py 78.6% <78.57%> (+4.47%) :arrow_up:
aiosip/application.py 72.15% <87.5%> (-5.13%) :arrow_down:
aiosip/utils.py 43.58% <0%> (-23.08%) :arrow_down:
aiosip/contact.py 67.14% <0%> (-11.43%) :arrow_down:
aiosip/message.py 73.8% <0%> (-3.81%) :arrow_down:
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update af98795...2dd0ceb. Read the comment docs.

vodik commented 6 years ago

I'm missing something here, what updates the dialog structure with the To tag once its been set?

ovv commented 6 years ago

https://github.com/Eyepea/aiosip/pull/112/files#diff-40bd7966cd02c1c7d15d5feea86ae769R64 https://github.com/Eyepea/aiosip/pull/112/files#diff-40bd7966cd02c1c7d15d5feea86ae769R318 https://github.com/Eyepea/aiosip/pull/112/files#diff-40bd7966cd02c1c7d15d5feea86ae769R449

Basically in the receive_message / receive_response of the dialogs

ovv commented 6 years ago

Do not merge yet. I'd like to do some more test after the rebase

ovv commented 6 years ago

So the tag gets added as part of the inbound processing? Okay, I think that might should be okay. Might be a little surprising if you dump the message as you don't see the original message anymore, but should be okay.

Yeah I thought about that. In the future the original message To and From should be a snapshot of the dialog To / From. That way we can update the details without changing the original message.

vodik commented 6 years ago

I think it might make sense to hide the original message and pull useful information out of the Request object - at least, that was my plan. I'm only half way there, which is why I left that class embedded right inside the dialog logic.

And in that case, it might make it easier to refactor if its turns out we can't put the tag on the message immediately - have it in the Request object instead.

I mean, in theory, we can do SIP forking (see #100) inside a single endpoint - an INVITE comes in and we send two distinct responses each with a different tag. Maybe the best API for this is to be able to fork the request object....

vodik commented 6 years ago

@moises-silva ^ ping if you don't mind.

ovv commented 6 years ago

Removed some more proxy stuff since it's not working. I'll look into it in a future PR