fix: Normalize endpoint and web _router_completed calls and tests #577
Conversation
Current coverage is 100% (diff: 100%)@@ master #577 diff @@
====================================
Files 42 42
Lines 8775 8791 +16
Methods 0 0
Messages 0 0
Branches 0 0
====================================
+ Hits 8775 8791 +16
Misses 0 0
Partials 0 0
|
|
Which way is it supposed to be? The endpoint.py change to remove the del statement was done first, so I had thought the endpoint.py change was going to propagate to webpush.py, not vice versa. Is it supposed to delete the router_data if its not set, or not? |
jrconlin
force-pushed
the
bug/549
branch
3 times, most recently
from
c05ecbd to
aa1ce41
Compare
| # it is impossible for us to reach them. | ||
| if not (router_token == router_data['token'] == self.senderID): | ||
| if not (senderid == self.senderID): |
There was a problem hiding this comment.
router['token'] is the registrationID and should NOT match senderid. I'm an idiot.
jrconlin
force-pushed
the
bug/549
branch
3 times, most recently
from
e0d75dd to
f4272db
Compare
| @@ -22,39 +21,45 @@ def __init__(self, ap_settings, router_conf): | |||
| self.min_ttl = router_conf.get("ttl", 60) | |||
| self.dryRun = router_conf.get("dryrun", False) | |||
| self.collapseKey = router_conf.get("collapseKey", "simplepush") | |||
| self.senderIDs = router_conf.get("senderIDs") | |||
| self.gcm = {} | |||
| self.senderIDs = {} | |||
There was a problem hiding this comment.
You could actually combine the gcm and senderIDs dicts into one at this point, {sid: (auth, GCM()),} .. but wait a second..
There was a problem hiding this comment.
well, really, I could create a map of {sid: GCM()} Holding onto the auth key is probably not needed. I divided things up for now mostly to keep it clear (at least for me). We can collapse things down if we need to.
|
r+ |
| uaid_data["connected_at"] = ms_time() | ||
| rt = self.valid_input.get( | ||
| 'subscription', {}).get('user_data', {}).get('router_type') | ||
| uaid_data["router_type"] = uaid_data.get("router_type", rt) |
There was a problem hiding this comment.
What's the rationale for a router being able to change the router type?
There was a problem hiding this comment.
We're setting it if it's not already set. (hrm, thought that i was hitting a test where that happened, but just re-ran and it passed. pulling it.)
There was a problem hiding this comment.
How would we get to this point if it wasn't set? The validation logic will toss out the request if there's no router_type before starting this process.
|
r-, need some clarification on what's occuring with router type resets, and get calls that aren't needed. |
Some android clients have an older SenderID. The server needs to allow and send to that older SenderID since it is extremely difficult to change this value on the client. In addtion: * drops user on bridge update with empty new routing data (there's nothing to route to, so drop the user) * fixes comments and var names to be more expicit about what's happening. * closes #549, #579
|
r+ fe0d19c |
closes #549
@bbangert r?