Skip to content

Re-save installation is allowed if LDS is enabled #607

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 2 commits into from
Apr 21, 2017
Merged

Re-save installation is allowed if LDS is enabled #607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed48e36
Allowing to re-save installation if LDS is enabled
hermanliang Mar 20, 2017
6a2abc2
handle MissingRequiredFieldError exception (code 135)
hermanliang Apr 19, 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
12 changes: 11 additions & 1 deletion Parse/src/main/java/com/parse/OfflineStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,17 @@ public Integer then(Task<List<T>> task) throws Exception {
if (oldObjectId.equals(newObjectId)) {
return;
}
throw new RuntimeException("objectIds cannot be changed in offline mode.");
/**
* Special case for re-saving installation if it was deleted on the server
* @see ParseInstallation#saveAsync(String, Task)
*/
if (object instanceof ParseInstallation
&& newObjectId == null) {
classNameAndObjectIdToObjectMap.remove(Pair.create(object.getClassName(), oldObjectId));
return;
} else {
throw new RuntimeException("objectIds cannot be changed in offline mode.");
}
}

String className = object.getClassName();
Expand Down
5 changes: 5 additions & 0 deletions Parse/src/main/java/com/parse/ParseException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ public class ParseException extends Exception {
*/
public static final int INVALID_EMAIL_ADDRESS = 125;

/**
* Error code indicating that required field is missing.
*/
public static final int MISSING_REQUIRED_FIELD_ERROR = 135;

/**
* Error code indicating that a unique field was given a value that is already taken.
*/
Expand Down
19 changes: 10 additions & 9 deletions Parse/src/main/java/com/parse/ParseInstallation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,16 @@ public Task<T> then(Task<Void> task) throws Exception {
@Override
public Task<Void> then(Task<Void> task) throws Exception {
// Retry the fetch as a save operation because this Installation was deleted on the server.
// Do not attempt to resave an object if LDS is enabled, since changing objectId is not allowed.
Copy link
Contributor

Choose a reason for hiding this comment

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

So is it fairly easy to null out your objectId() on a failure and then cause multiple install id's per app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the ParseException.OBJECT_NOT_FOUND (line 148) can trigger the re-creating process.
If just null out the objectId and save installation will not create another installation data. (It's weird, I'm still figuring out why)
BTW, I think setObjectId() should not be a public method, package private may be more appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha line 148 is indeed key.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw this line can be removed, there is instanceof below

if(!Parse.isLocalDatastoreEnabled()
&& task.getError() != null
&& task.getError() instanceof ParseException
&& ((ParseException) task.getError()).getCode() == ParseException.OBJECT_NOT_FOUND) {
synchronized (mutex) {
setState(new State.Builder(getState()).objectId(null).build());
markAllFieldsDirty();
return ParseInstallation.super.saveAsync(sessionToken, toAwait);
if (task.getError() != null
&& task.getError() instanceof ParseException) {
int errCode = ((ParseException) task.getError()).getCode();
if (errCode == ParseException.OBJECT_NOT_FOUND
|| (errCode == ParseException.MISSING_REQUIRED_FIELD_ERROR && getObjectId() == null)) {
synchronized (mutex) {
setState(new State.Builder(getState()).objectId(null).build());
markAllFieldsDirty();
return ParseInstallation.super.saveAsync(sessionToken, toAwait);
}
}
}
return task;
Expand Down
65 changes: 54 additions & 11 deletions Parse/src/test/java/com/parse/ParseInstallationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,24 +121,48 @@ public void testInstallationObjectIdCannotBeChanged() throws Exception {
}

@Test
public void testSaveAsync() throws Exception {
public void testMissingRequiredFieldWhenSaveAsync() throws Exception {
String sessionToken = "sessionToken";
Task<Void> toAwait = Task.forResult(null);

ParseCurrentInstallationController controller =
mock(ParseCurrentInstallationController.class);
ParseInstallation currentInstallation = new ParseInstallation();
when(controller.getAsync())
.thenReturn(Task.forResult(currentInstallation));
ParseCurrentInstallationController controller = mockCurrentInstallationController();

ParseObjectController objController = mock(ParseObjectController.class);
// mock return task when Installation was deleted on the server
Task<ParseObject.State> taskError = Task.forError(new ParseException(ParseException.MISSING_REQUIRED_FIELD_ERROR, ""));
// mock return task when Installation was re-saved to the server
Task<ParseObject.State> task = Task.forResult(null);
when(objController.saveAsync(
any(ParseObject.State.class),
any(ParseOperationSet.class),
eq(sessionToken),
any(ParseDecoder.class)))
.thenReturn(taskError)
.thenReturn(task);
ParseCorePlugins.getInstance()
.registerCurrentInstallationController(controller);
ParseObject.State state = new ParseObject.State.Builder("_Installation")
.put("deviceToken", "deviceToken")
.build();
.registerObjectController(objController);

ParseInstallation installation = ParseInstallation.getCurrentInstallation();
assertNotNull(installation);
installation.setState(state);
installation.put("key", "value");
installation.saveAsync(sessionToken, toAwait);
verify(controller).getAsync();
verify(objController, times(2)).saveAsync(
any(ParseObject.State.class),
any(ParseOperationSet.class),
eq(sessionToken),
any(ParseDecoder.class));
}

@Test
public void testObjectNotFoundWhenSaveAsync() throws Exception {
OfflineStore lds = new OfflineStore(RuntimeEnvironment.application);
Parse.setLocalDatastore(lds);

String sessionToken = "sessionToken";
Task<Void> toAwait = Task.forResult(null);

ParseCurrentInstallationController controller = mockCurrentInstallationController();
ParseObjectController objController = mock(ParseObjectController.class);
// mock return task when Installation was deleted on the server
Task<ParseObject.State> taskError = Task.forError(new ParseException(ParseException.OBJECT_NOT_FOUND, ""));
Expand All @@ -154,6 +178,14 @@ public void testSaveAsync() throws Exception {
ParseCorePlugins.getInstance()
.registerObjectController(objController);

ParseObject.State state = new ParseObject.State.Builder("_Installation")
.objectId("oldId")
.put("deviceToken", "deviceToken")
.build();
ParseInstallation installation = ParseInstallation.getCurrentInstallation();
assertNotNull(installation);
installation.setState(state);
installation.put("key", "value");
installation.saveAsync(sessionToken, toAwait);

verify(controller).getAsync();
Expand Down Expand Up @@ -355,4 +387,15 @@ private static void mocksForUpdateBeforeSave() {
when(plugins.applicationContext()).thenReturn(RuntimeEnvironment.application);
ParsePlugins.set(plugins);
}

private ParseCurrentInstallationController mockCurrentInstallationController() {
ParseCurrentInstallationController controller =
mock(ParseCurrentInstallationController.class);
ParseInstallation currentInstallation = new ParseInstallation();
when(controller.getAsync())
.thenReturn(Task.forResult(currentInstallation));
ParseCorePlugins.getInstance()
.registerCurrentInstallationController(controller);
return controller;
}
}