Skip to content

Add client support for scheduling push notifications #675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 3, 2017
Merged

Add client support for scheduling push notifications #675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aba69a2
Add client support for push_time
natario1 May 31, 2017
2854fbf
Tests for setPushTime
natario1 Jun 1, 2017
7d81aa5
Fix pushTime tests
natario1 Jun 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Parse/src/main/java/com/parse/ParsePush.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ private static void checkArgument(boolean expression, Object errorMessage) {
private ParseQuery<ParseInstallation> query;
private Long expirationTime;
private Long expirationTimeInterval;
private Long pushTime;
private Boolean pushToIOS;
private Boolean pushToAndroid;
private JSONObject data;
Expand All @@ -72,6 +73,7 @@ public Builder(State state) {
: new ParseQuery<>(new ParseQuery.State.Builder<ParseInstallation>(state.queryState()));
this.expirationTime = state.expirationTime();
this.expirationTimeInterval = state.expirationTimeInterval();
this.pushTime = state.pushTime();
this.pushToIOS = state.pushToIOS();
this.pushToAndroid = state.pushToAndroid();
// Since in state.build() we check data is not null, we do not need to check it again here.
Expand All @@ -96,6 +98,18 @@ public Builder expirationTimeInterval(Long expirationTimeInterval) {
return this;
}

public Builder pushTime(Long pushTime) {
if (pushTime != null) {
long now = System.currentTimeMillis() / 1000;
long twoWeeks = 60*60*24*7*2;
checkArgument(pushTime > now, "Scheduled push time can not be in the past");
checkArgument(pushTime < now + twoWeeks, "Scheduled push time can not be more than " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the two week restriction something imposed by the server?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes see here. Same code is in iOS version of this feature so it should be ok

"two weeks in the future");
}
this.pushTime = pushTime;
return this;
}

public Builder pushToIOS(Boolean pushToIOS) {
checkArgument(query == null, "Cannot set push targets (i.e. setPushToAndroid or " +
"setPushToIOS) when pushing to a query");
Expand Down Expand Up @@ -151,6 +165,7 @@ public State build() {
private final ParseQuery.State<ParseInstallation> queryState;
private final Long expirationTime;
private final Long expirationTimeInterval;
private final Long pushTime;
private final Boolean pushToIOS;
private final Boolean pushToAndroid;
private final JSONObject data;
Expand All @@ -161,6 +176,7 @@ private State(Builder builder) {
this.queryState = builder.query == null ? null : builder.query.getBuilder().build();
this.expirationTime = builder.expirationTime;
this.expirationTimeInterval = builder.expirationTimeInterval;
this.pushTime = builder.pushTime;
this.pushToIOS = builder.pushToIOS;
this.pushToAndroid = builder.pushToAndroid;
// Since in builder.build() we check data is not null, we do not need to check it again here.
Expand Down Expand Up @@ -189,6 +205,10 @@ public Long expirationTimeInterval() {
return expirationTimeInterval;
}

public Long pushTime() {
return pushTime;
}

public Boolean pushToIOS() {
return pushToIOS;
}
Expand Down Expand Up @@ -421,6 +441,14 @@ public void clearExpiration() {
builder.expirationTimeInterval(null);
}

/**
* Sets a UNIX epoch timestamp at which this notification should be delivered, in seconds (UTC).
* Scheduled time can not be in the past and must be at most two weeks in the future.
*/
public void setPushTime(long time) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be public void setPushTime(Date date)?
(pushTime = date.getTime()/1000)

or change the time unit to milliseconds to match Java's timestamp
(pushTime = time/1000)

Because unit of UNIX epoch timestamp is seconds, but developers probably using Date.getTime() or System.currentTimeMillis() to get timestamp (unit is milliseconds), then they will fail if not converted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah seconds are a terrible choice, but we already have setExpirationTime(long) and setExpirationInterval(long) in seconds, it would be confusing to have this in millis or in Date.
Note sure what to do, maybe we should add a Date version for all three methods. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, I didn't notice that. We should use the same time unit (seconds) for all relevant methods.
I think it's okay to add a Date version.

builder.pushTime(time);
}

/**
* Set whether this push notification will go to iOS devices.
* <p/>
Expand Down
3 changes: 2 additions & 1 deletion Parse/src/main/java/com/parse/ParsePushController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public Task<Void> sendInBackground(ParsePush.State state, String sessionToken) {
}
}
return ParseRESTPushCommand.sendPushCommand(state.queryState(), state.channelSet(), deviceType,
state.expirationTime(), state.expirationTimeInterval(), state.data(), sessionToken);
state.expirationTime(), state.expirationTimeInterval(), state.pushTime(), state.data(),
sessionToken);
}
}
8 changes: 7 additions & 1 deletion Parse/src/main/java/com/parse/ParseRESTPushCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
/* package */ final static String KEY_DEVICE_TYPE = "deviceType";
/* package */ final static String KEY_EXPIRATION_TIME = "expiration_time";
/* package */ final static String KEY_EXPIRATION_INTERVAL = "expiration_interval";
/* package */ final static String KEY_PUSH_TIME = "push_time";
/* package */ final static String KEY_DATA = "data";

public ParseRESTPushCommand(
Expand All @@ -35,7 +36,7 @@ public ParseRESTPushCommand(

public static ParseRESTPushCommand sendPushCommand(ParseQuery.State<ParseInstallation> query,
Set<String> targetChannels, String targetDeviceType, Long expirationTime,
Long expirationInterval, JSONObject payload, String sessionToken) {
Long expirationInterval, Long pushTime, JSONObject payload, String sessionToken) {
JSONObject parameters = new JSONObject();
try {
if (targetChannels != null) {
Expand Down Expand Up @@ -63,9 +64,14 @@ public static ParseRESTPushCommand sendPushCommand(ParseQuery.State<ParseInstall
parameters.put(KEY_EXPIRATION_INTERVAL, expirationInterval);
}

if (pushTime != null) {
parameters.put(KEY_PUSH_TIME, pushTime);
}

if (payload != null) {
parameters.put(KEY_DATA, payload);
}

} catch (JSONException e) {
throw new RuntimeException(e);
}
Expand Down
37 changes: 37 additions & 0 deletions Parse/src/test/java/com/parse/ParsePushControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public void testBuildRESTSendPushCommandWithChannelSet() throws Exception {

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
// Verify device type and query
Expand Down Expand Up @@ -113,6 +114,7 @@ public void testBuildRESTSendPushCommandWithExpirationTime() throws Exception {

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_CHANNELS));
// Verify device type and query
Expand All @@ -123,6 +125,36 @@ public void testBuildRESTSendPushCommandWithExpirationTime() throws Exception {
assertEquals(1400000000, jsonParameters.getLong(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
}

@Test
public void testBuildRESTSendPushCommandWithPushTime() throws Exception {
ParseHttpClient restClient = mock(ParseHttpClient.class);
ParsePushController controller = new ParsePushController(restClient);

// Build PushState
JSONObject data = new JSONObject();
data.put(ParsePush.KEY_DATA_MESSAGE, "hello world");
long pushTime = System.currentTimeMillis() / 1000 + 1000;
ParsePush.State state = new ParsePush.State.Builder()
.data(data)
.pushTime(pushTime)
.build();

// Build command
ParseRESTCommand pushCommand = controller.buildRESTSendPushCommand(state, "sessionToken");

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_CHANNELS));
// Verify device type and query
assertEquals("{}", jsonParameters.get(ParseRESTPushCommand.KEY_WHERE).toString());
assertEquals("hello world",
jsonParameters.getJSONObject(ParseRESTPushCommand.KEY_DATA)
.getString(ParsePush.KEY_DATA_MESSAGE));
assertEquals(pushTime, jsonParameters.getLong(ParseRESTPushCommand.KEY_PUSH_TIME));
}

@Test
public void testBuildRESTSendPushCommandWithExpirationTimeInterval() throws Exception {
ParseHttpClient restClient = mock(ParseHttpClient.class);
Expand All @@ -141,6 +173,7 @@ public void testBuildRESTSendPushCommandWithExpirationTimeInterval() throws Exce

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_CHANNELS));
// Verify device type and query
Expand Down Expand Up @@ -172,6 +205,7 @@ public void testBuildRESTSendPushCommandWithQuery() throws Exception {

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
assertFalse(jsonParameters.getJSONObject(ParseRESTPushCommand.KEY_WHERE)
Expand Down Expand Up @@ -206,6 +240,7 @@ public void testBuildRESTSendPushCommandWithPushToAndroid() throws Exception {

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_CHANNELS));
Expand Down Expand Up @@ -235,6 +270,7 @@ public void testBuildRESTSendPushCommandWithPushToIOS() throws Exception {

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_CHANNELS));
Expand Down Expand Up @@ -265,6 +301,7 @@ public void testBuildRESTSendPushCommandWithPushToIOSAndAndroid() throws Excepti

// Verify command
JSONObject jsonParameters = pushCommand.jsonParameters;
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_PUSH_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_TIME));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_EXPIRATION_INTERVAL));
assertFalse(jsonParameters.has(ParseRESTPushCommand.KEY_CHANNELS));
Expand Down
52 changes: 52 additions & 0 deletions Parse/src/test/java/com/parse/ParsePushStateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public void testDefaultsWithData() throws Exception {

assertEquals(null, state.expirationTime());
assertEquals(null, state.expirationTimeInterval());
assertEquals(null, state.pushTime());
assertEquals(null, state.channelSet());
JSONAssert.assertEquals(data, state.data(), JSONCompareMode.NON_EXTENSIBLE);
assertEquals(null, state.pushToAndroid());
Expand All @@ -68,6 +69,7 @@ public void testCopy() throws JSONException {
ParsePush.State state = mock(ParsePush.State.class);
when(state.expirationTime()).thenReturn(1L);
when(state.expirationTimeInterval()).thenReturn(2L);
when(state.pushTime()).thenReturn(3L);
Set channelSet = Sets.newSet("one", "two");
when(state.channelSet()).thenReturn(channelSet);
JSONObject data = new JSONObject();
Expand All @@ -82,6 +84,7 @@ public void testCopy() throws JSONException {
ParsePush.State copy = new ParsePush.State.Builder(state).build();
assertSame(1L, copy.expirationTime());
assertSame(2L, copy.expirationTimeInterval());
assertSame(3L, copy.pushTime());
Set channelSetCopy = copy.channelSet();
assertNotSame(channelSet, channelSetCopy);
assertTrue(channelSetCopy.size() == 2 && channelSetCopy.contains("one"));
Expand Down Expand Up @@ -151,6 +154,55 @@ public void testExpirationTimeIntervalNormalInterval() {

//endregion

//region testPushTime

@Test
public void testPushTimeNullTime() {
ParsePush.State.Builder builder = new ParsePush.State.Builder();

ParsePush.State state = builder
.pushTime(null)
.data(new JSONObject())
.build();

assertEquals(null, state.pushTime());
}

@Test
public void testPushTimeNormalTime() {
ParsePush.State.Builder builder = new ParsePush.State.Builder();

long time = System.currentTimeMillis() / 1000 + 1000;
ParsePush.State state = builder
.pushTime(time)
.data(new JSONObject())
.build();

assertEquals(time, state.pushTime().longValue());
}

@Test(expected = IllegalArgumentException.class)
public void testPushTimeInThePast() {
ParsePush.State.Builder builder = new ParsePush.State.Builder();

ParsePush.State state = builder
.pushTime(System.currentTimeMillis() / 1000 - 1000)
.data(new JSONObject())
.build();
}

@Test(expected = IllegalArgumentException.class)
public void testPushTimeTwoWeeksFromNow() {
ParsePush.State.Builder builder = new ParsePush.State.Builder();

ParsePush.State state = builder
.pushTime(System.currentTimeMillis() / 1000 + 60*60*24*7*3)
.data(new JSONObject())
.build();
}

//endregion

//region testChannelSet

@Test(expected = IllegalArgumentException.class)
Expand Down
20 changes: 20 additions & 0 deletions Parse/src/test/java/com/parse/ParsePushTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,26 @@ public void testClearExpiration() {

//endregion

//region testSetPushTime

// We only test a basic case here to make sure logic in ParsePush is correct, more comprehensive
// builder test cases should be in ParsePushState test
@Test
public void testSetPushTime() throws Exception {
ParsePush push = new ParsePush();
long time = System.currentTimeMillis() / 1000 + 1000;
push.setPushTime(time);

// Right now it is hard for us to test a builder, so we build a state to test the builder is
// set correctly
// We have to set message otherwise build() will throw an exception
push.setMessage("message");
ParsePush.State state = push.builder.build();
assertEquals(time, state.pushTime().longValue());
}

//endregion

//region testSetPushToIOS

// We only test a basic case here to make sure logic in ParsePush is correct, more comprehensive
Expand Down