feat: add support for custom objectId

[martinpfannemueller]

I implemented the custom objectId feature mentioned in #1087 including unit tests. Is there something missing to get this merged?

Thanks!

[mtrezza]

Looking at the changes, should we add a lint step to the CI?

[azlekov]

Please open an issue @mtrezza I can take it

[mtrezza]

Great, there it is! https://github.com/parse-community/Parse-SDK-Android/issues/1119

[parse-github-assistant[bot]]

I will reformat the title to use the proper commit message syntax.

[parse-github-assistant[bot]]

Thanks for opening this pull request!

• ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

[martinpfannemueller]

I have no idea why the two tests testObjectNotFoundWhenSaveAsync and testMissingRequiredFieldWhenSaveAsync in the ParseInstallationTest-file fail here. Locally everything runs just fine. Attached you will find the output of running the task jacocoTestReport locally. jacocoTestReport.log

[azlekov]

@martinpfannemueller can you close and re-open the PR so we can trigger another CI

[azlekov]

@mtrezza how we can re-run the CI build. I tried already closing and re-opening the issue.

[mtrezza]

I suggest to first resolve the conflicts, then we look into the CI.

[martinpfannemueller]

I just merged the changes from the master. Again, locally everything runs just fine.

[codecov[bot]]

Codecov Report

Merging #1088 (461a8db) into master (fbe8d87) will increase coverage by 0.16%. The diff coverage is 75.00%.

@@             Coverage Diff              @@
##             master    #1088      +/-   ##
============================================
+ Coverage     66.11%   66.27%   +0.16%     
- Complexity     2241     2251      +10     
============================================
  Files           122      122              
  Lines          9965     9983      +18     
  Branches       1339     1343       +4     
============================================
+ Hits           6588     6616      +28     
+ Misses         2860     2850      -10     
  Partials        517      517              

Impacted Files	Coverage Δ
parse/src/main/java/com/parse/ParseObject.java	61.35% <50.00%> (+0.90%)	:arrow_up:
...rc/main/java/com/parse/ParseRESTObjectCommand.java	84.61% <62.50%> (-10.39%)	:arrow_down:
parse/src/main/java/com/parse/Parse.java	61.39% <100.00%> (+1.49%)	:arrow_up:
.../src/main/java/com/parse/ParseEventuallyQueue.java	10.95% <0.00%> (+1.36%)	:arrow_up:

---

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 bd3ac1d...461a8db. Read the comment docs.

[mtrezza]

Could you please again merge master into this? There have been some commits.

[azlekov]

@martinpfannemueller can you please run ./gradlew spotlessApply and commit the formatted files. This should fix the CI

[martinpfannemueller]

Still the same problem. This maybe has to do something with the OS or the development environment then. I am running the tests inside Android Studio on macOS. In general, do you have an idea how we can debug the two failing tests in the CI pipeline? When I have time tomorrow, I could try the tests on an Ubuntu installation as well.

[azlekov]

@martinpfannemueller I dig a little bit. The tests looks to fail for a reason regarding changes you have introduced. There two failing tests:

com.parse.ParseInstallationTest > testObjectNotFoundWhenSaveAsync FAILED
    com.parse.ParseException at ParseObject.java:2250

In ParseObject line 2250 you can find:

if (Parse.isAllowCustomObjectId() && getObjectId() == null) {
    return Task.forError(new ParseException(104, "ObjectId must not be null"));
}

The second failing test is

com.parse.ParseInstallationTest > testMissingRequiredFieldWhenSaveAsync FAILED
    com.parse.ParseException at ParseInstallationTest.java:163

There inside the ParseInstallationTest you can find

ParseTaskUtils.wait(installation.saveAsync(sessionToken, toAwait));

For me it looks like the tests are not mocked well and when save the work with ParseInstallation and try to save it throws the exception you have introduced. Can you check the failing tests and mock them properly. Maybe the Parse instance is not set properly and the custom object id is allowed, cannot say. Unfortunately the tests passed on my macOS as well, so let's try to patch this for the GitHub CI.

[martinpfannemueller]

I just tried to update the unit tests as it was partly possible to reproduce the failing tests. It definitely has something to do with the Parse instance. Therefore, I tried to clean them up properly now in the new tests at the end.

EDIT: I set up a build environment using Ubuntu. Even though most of the time there are no failing tests anymore, it still happens that the two mentioned tests fail. It seems that it still matters in what order the tests are executed. As I basically tried to build the tests as the others, I do not know what could be wrong now. Maybe it will work now in the CI as well.

It would definitely help if you could provide some guidelines how the mocking and the different plugins as well as the different states of the mocked objects work together in the tests, e.g., by providing a sample when using the local datastore or when a specific response is expected.

[mtrezza]

It would definitely help if you could provide some guidelines how the mocking and the different plugins as well as the different states of the mocked objects work together in the tests, e.g., by providing a sample when using the local datastore or when a specific response is expected.

@L3K0V @Jawnnypoo could you do that or should I look into it?

[Jawnnypoo]

I don't have context into that, sorry.

[martinpfannemueller]

I added a Parse.destroy() call to the ParseObjectTests as well as reset the state variable allowCustomObjectId on destroy. Now the tests also run in my Linux environment.

[martinpfannemueller]

I added an explicit test for the ParseRESTCommand and the logic of it. The test coverage should be fine now.

[mtrezza]

Running tests...

[azlekov]

@martinpfannemueller gentle ping when you have some free time if is possible to add some JavaDoc as @mtrezza pleases, so this can be merged. Looking forward to see this merged 🤞

[martinpfannemueller]

I just added some JavaDoc to allowCustomObjectId() in the configuration builder and the isAllowCustomObjectId() getter.

[mtrezza]

There also seems to be a lint fix needed, see the failing lint check

[martinpfannemueller]

I updated the JavaDoc and ran spotlessApply

[mtrezza]

Thanks, let just see whether the CI passes.

[parseplatformorg]

🎉 This change has been released in version 2.1.0