Skip to content

Commit 678f5f9

Browse files
committed
Fixes Redth#146
The events for expired or changed subscription Id's were not being fired in the BasePushService class for GCM (and the one for C2DM). This is now fixed along with some tests to validate.
1 parent 76a2c9c commit 678f5f9

File tree

5 files changed

+184
-14
lines changed

5 files changed

+184
-14
lines changed

PushSharp.Android/C2dm/C2dmPushChannel.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ private void ProcessResponse(C2dmAsyncParameters asyncParam, C2dmMessageTranspor
263263
else if (response.ResponseStatus == MessageTransportResponseStatus.InvalidRegistration || response.ResponseStatus == MessageTransportResponseStatus.NotRegistered)
264264
{
265265
//Device subscription is no good!
266-
asyncParam.Callback(this, new SendNotificationResult(response.Message, false, new DeviceSubscriptonExpiredException()) { IsSubscriptionExpired = true});
266+
asyncParam.Callback(this, new SendNotificationResult(response.Message, false, new DeviceSubscriptonExpiredException()) { OldSubscriptionId = response.Message.RegistrationId, IsSubscriptionExpired = true});
267267
}
268268
else
269269
{

PushSharp.Android/Gcm/GcmPushChannel.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -234,16 +234,21 @@ void processResponseOk(GcmAsyncParameters asyncParam)
234234
if (singleResultNotification.RegistrationIds != null && singleResultNotification.RegistrationIds.Count > 0)
235235
oldRegistrationId = singleResultNotification.RegistrationIds[0];
236236

237-
asyncParam.Callback(this, new SendNotificationResult(singleResultNotification, false, new DeviceSubscriptonExpiredException()) { OldSubscriptionId = oldRegistrationId, NewSubscriptionId = newRegistrationId });
237+
asyncParam.Callback(this, new SendNotificationResult(singleResultNotification, false, new DeviceSubscriptonExpiredException()) { OldSubscriptionId = oldRegistrationId, NewSubscriptionId = newRegistrationId, IsSubscriptionExpired = true });
238238
}
239239
else if (r.ResponseStatus == GcmMessageTransportResponseStatus.Unavailable)
240240
{
241241
asyncParam.Callback(this, new SendNotificationResult(singleResultNotification, true, new Exception("Unavailable Response Status")));
242242
}
243243
else if (r.ResponseStatus == GcmMessageTransportResponseStatus.NotRegistered)
244244
{
245+
var oldRegistrationId = string.Empty;
246+
247+
if (singleResultNotification.RegistrationIds != null && singleResultNotification.RegistrationIds.Count > 0)
248+
oldRegistrationId = singleResultNotification.RegistrationIds[0];
249+
245250
//Raise failure and device expired
246-
asyncParam.Callback(this, new SendNotificationResult(singleResultNotification, false, new DeviceSubscriptonExpiredException()));
251+
asyncParam.Callback(this, new SendNotificationResult(singleResultNotification, false, new DeviceSubscriptonExpiredException()) { OldSubscriptionId = oldRegistrationId, IsSubscriptionExpired = true, SubscriptionExpiryUtc = DateTime.UtcNow });
247252
}
248253
else
249254
{

PushSharp.Core/PushServiceBase.cs

+25-11
Original file line numberDiff line numberDiff line change
@@ -343,19 +343,33 @@ private void DoChannelWork(IPushChannel channel, CancellationTokenSource cancelT
343343
}
344344
else
345345
{
346-
//This is a fairly special case that only GCM should really ever raise
347-
if (!string.IsNullOrEmpty(result.NewSubscriptionId) && !string.IsNullOrEmpty(result.OldSubscriptionId))
348-
{
349-
var evt = this.OnDeviceSubscriptionChanged;
350-
if (evt != null)
351-
evt(this, result.OldSubscriptionId, result.NewSubscriptionId, result.Notification);
352-
}
353-
346+
//Result was a success, but there are still more possible outcomes than an outright success
354347
if (!result.IsSuccess)
355348
{
356-
var evt = this.OnNotificationFailed;
357-
if (evt != null)
358-
evt(this, result.Notification, result.Error);
349+
//Check if the subscription was expired
350+
if (result.IsSubscriptionExpired)
351+
{
352+
//If there is a new id, the subscription must have changed
353+
//This is a fairly special case that only GCM should really ever raise
354+
if (!string.IsNullOrEmpty(result.NewSubscriptionId))
355+
{
356+
var evt = this.OnDeviceSubscriptionChanged;
357+
if (evt != null)
358+
evt(this, result.OldSubscriptionId, result.NewSubscriptionId, result.Notification);
359+
}
360+
else
361+
{
362+
var evt = this.OnDeviceSubscriptionExpired;
363+
if (evt != null)
364+
evt(this, result.OldSubscriptionId, result.SubscriptionExpiryUtc, result.Notification);
365+
}
366+
}
367+
else //Otherwise some general failure
368+
{
369+
var evt = this.OnNotificationFailed;
370+
if (evt != null)
371+
evt(this, result.Notification, result.Error);
372+
}
359373
}
360374
else
361375
{

PushSharp.Core/SendNotificationResult.cs

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public SendNotificationResult(INotification notification, bool shouldRequeue = f
2222
public bool CountsAsRequeue { get; set; }
2323
public string OldSubscriptionId { get; set; }
2424
public string NewSubscriptionId { get; set; }
25+
public DateTime SubscriptionExpiryUtc { get;set; }
2526
public bool IsSubscriptionExpired { get; set; }
2627
public bool IsSuccess { get { return Error == null; } }
2728
}

Tests/PushSharp.Tests/GcmTests.cs

+150
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,86 @@ public void GCM_Batched_Single_ShouldSucceed()
110110
TestNotifications(false, 1, 1, 0);
111111
}
112112

113+
[Test]
114+
public void GCM_Subscription_ShouldBeExpired()
115+
{
116+
int msgIdOn = 1;
117+
118+
int pushFailCount = 0;
119+
int pushSuccessCount = 0;
120+
int subChangedCount = 0;
121+
int subExpiredCount = 0;
122+
123+
var notifications = new List<GcmNotification>() {
124+
new GcmNotification().ForDeviceRegistrationId("NOTREGISTERED").WithJson(@"{""key"":""value""}")
125+
};
126+
127+
TestNotifications(notifications,
128+
new List<GcmMessageResponseFilter>() {
129+
new GcmMessageResponseFilter()
130+
{
131+
IsMatch = (request, s) => {
132+
return s.Equals("NOTREGISTERED", StringComparison.InvariantCultureIgnoreCase);
133+
},
134+
Status = new GcmMessageResult() {
135+
ResponseStatus = GcmMessageTransportResponseStatus.NotRegistered,
136+
MessageId = "1:" + msgIdOn++
137+
}
138+
}
139+
},
140+
(sender, notification) => pushSuccessCount++, //Success
141+
(sender, notification, error) => pushFailCount++, //Failed
142+
(sender, oldId, newId, notification) => subChangedCount++,
143+
(sender, id, expiryDate, notification) => subExpiredCount++
144+
);
145+
146+
Assert.AreEqual(0, pushFailCount, "Client - Failed Count");
147+
Assert.AreEqual(0, pushSuccessCount, "Client - Success Count");
148+
Assert.AreEqual(0, subChangedCount, "Client - SubscriptionId Changed Count");
149+
Assert.AreEqual(notifications.Count, subExpiredCount, "Client - SubscriptionId Expired Count");
150+
}
151+
152+
153+
[Test]
154+
public void GCM_Subscription_ShouldBeChanged()
155+
{
156+
int msgIdOn = 1;
157+
158+
int pushFailCount = 0;
159+
int pushSuccessCount = 0;
160+
int subChangedCount = 0;
161+
int subExpiredCount = 0;
162+
163+
var notifications = new List<GcmNotification>() {
164+
new GcmNotification().ForDeviceRegistrationId("NOTREGISTERED").WithJson(@"{""key"":""value""}")
165+
};
166+
167+
TestNotifications(notifications,
168+
new List<GcmMessageResponseFilter>() {
169+
new GcmMessageResponseFilter()
170+
{
171+
IsMatch = (request, s) => {
172+
return s.Equals("NOTREGISTERED", StringComparison.InvariantCultureIgnoreCase);
173+
},
174+
Status = new GcmMessageResult() {
175+
ResponseStatus = GcmMessageTransportResponseStatus.NotRegistered,
176+
CanonicalRegistrationId = "NEWID",
177+
MessageId = "1:" + msgIdOn++
178+
}
179+
}
180+
},
181+
(sender, notification) => pushSuccessCount++, //Success
182+
(sender, notification, error) => pushFailCount++, //Failed
183+
(sender, oldId, newId, notification) => subChangedCount++,
184+
(sender, id, expiryDate, notification) => subExpiredCount++
185+
);
186+
187+
Assert.AreEqual(0, pushFailCount, "Client - Failed Count");
188+
Assert.AreEqual(0, pushSuccessCount, "Client - Success Count");
189+
Assert.AreEqual(notifications.Count, subChangedCount, "Client - SubscriptionId Changed Count");
190+
Assert.AreEqual(0, subExpiredCount, "Client - SubscriptionId Expired Count");
191+
}
192+
113193

114194
public void TestNotifications(bool shouldBatch, int toQueue, int expectSuccessful, int expectFailed, int[] indexesToFail = null)
115195
{
@@ -139,6 +219,16 @@ public void TestNotifications(bool shouldBatch, int toQueue, int expectSuccessfu
139219
}
140220
});
141221

222+
server.MessageResponseFilters.Add(new GcmMessageResponseFilter()
223+
{
224+
IsMatch = (request, s) => {
225+
return s.Equals("NOTREGISTERED", StringComparison.InvariantCultureIgnoreCase);
226+
},
227+
Status = new GcmMessageResult() {
228+
ResponseStatus = GcmMessageTransportResponseStatus.NotRegistered,
229+
MessageId = "1:" + msgIdOn++
230+
}
231+
});
142232
//var waitServerFinished = new ManualResetEvent(false);
143233

144234
server.Start(testPort, response =>
@@ -197,5 +287,65 @@ public void TestNotifications(bool shouldBatch, int toQueue, int expectSuccessfu
197287
Assert.AreEqual(expectFailed, pushFailCount, "Client - Failed Count");
198288
Assert.AreEqual(expectSuccessful, pushSuccessCount, "Client - Success Count");
199289
}
290+
291+
292+
293+
294+
public void TestNotifications(List<GcmNotification> notifications,
295+
List<GcmMessageResponseFilter> responseFilters,
296+
Action<object, INotification> sentCallback,
297+
Action<object, INotification, Exception> failedCallback,
298+
Action<object, string, string, INotification> subscriptionChangedCallback,
299+
Action<object, string, DateTime, INotification> subscriptionExpiredCallback)
300+
{
301+
testPort++;
302+
303+
int pushFailCount = 0;
304+
int pushSuccessCount = 0;
305+
306+
int serverReceivedCount = 0;
307+
int serverReceivedFailCount = 0;
308+
int serverReceivedSuccessCount = 0;
309+
310+
311+
var server = new TestServers.GcmTestServer();
312+
313+
server.MessageResponseFilters.AddRange(responseFilters);
314+
315+
server.Start(testPort, response => {
316+
serverReceivedCount += (int)response.NumberOfCanonicalIds;
317+
serverReceivedSuccessCount += (int) response.NumberOfSuccesses;
318+
serverReceivedFailCount += (int) response.NumberOfFailures;
319+
});
320+
321+
322+
323+
var settings = new GcmPushChannelSettings("SENDERAUTHTOKEN");
324+
settings.OverrideUrl("http://localhost:" + (testPort) + "/");
325+
326+
var push = new GcmPushService(settings);
327+
push.OnNotificationSent += (sender, notification1) => {
328+
pushSuccessCount++;
329+
sentCallback(sender, notification1);
330+
};
331+
push.OnNotificationFailed += (sender, notification1, error) => {
332+
pushFailCount++;
333+
failedCallback(sender, notification1, error);
334+
};
335+
push.OnDeviceSubscriptionChanged += (sender, oldSubscriptionId, newSubscriptionId, notification) => subscriptionChangedCallback(sender, oldSubscriptionId, newSubscriptionId, notification);
336+
push.OnDeviceSubscriptionExpired += (sender, expiredSubscriptionId, expirationDateUtc, notification) => subscriptionExpiredCallback(sender, expiredSubscriptionId, expirationDateUtc, notification);
337+
338+
339+
foreach (var n in notifications)
340+
push.QueueNotification(n);
341+
342+
push.Stop();
343+
push.Dispose();
344+
345+
server.Dispose();
346+
//waitServerFinished.WaitOne();
347+
348+
Console.WriteLine("TEST-> DISPOSE.");
349+
}
200350
}
201351
}

0 commit comments

Comments
 (0)