Skip to content
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

Jump to bottom

Fix NPE in ParseKeyValueCache due to cache dir not exist #141

Merged
merged 1 commit into from
Sep 18, 2015
Merged

Fix NPE in ParseKeyValueCache due to cache dir not exist #141

merged 1 commit into from
Sep 18, 2015

Conversation

wangmengyan95
Copy link
Contributor

  1. Fix ParseKeyValueCache.saveToKeyValueCache cacheDir.listFiles().length NPE.
  2. Refactor ParseKeyValueCache.saveToKeyValueCache
  3. Add unit tests

@wangmengyan95
Copy link
Contributor Author

The additional checking I add files == null(assume mkdir fails) is hard to reach in unit test, so probably that is why codecov test not pass.

@wangmengyan95 wangmengyan95 added this to the 1.10.3 milestone Sep 17, 2015
@wangmengyan95 wangmengyan95 mentioned this pull request Sep 17, 2015
Closed
grantland
grantland reviewed Sep 18, 2015
View reviewed changes
Parse/src/test/java/com/parse/ParseKeyValueCacheTest.java
@Test
public void testSaveToKeyValueCacheWithoutCacheDir() throws Exception {
// Delete the cache folder(Simulate users clear the app cache)
File keyValueCacheDir = new File(temporaryFolder.getRoot(), "ParseKeyValueCache");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It would be better to hold a reference to the File we use in initialize() in setUp and use that here.

@grantland grantland assigned wangmengyan95 and unassigned grantland Sep 18, 2015
@grantland
Copy link
Contributor

LGTM, small nit

Also, it looks like ParseFileController might also be affected by a similar problem.

@wangmengyan95
Copy link
Contributor Author

BTW, @grantland testMultipleAsynchronousWrites seems really wried, there is no assert in the test case, is it intended?

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.t8376668_NPE_in_ParseKeyValueCache branch from 6ca721a to e8ebbc7 Compare September 18, 2015 19:32
wangmengyan95 added a commit that referenced this pull request Sep 18, 2015
@wangmengyan95
Merge pull request #141 from ParsePlatform/wangmengyan.t8376668_NPE_i…
da739de 
…n_ParseKeyValueCache

Fix NPE in ParseKeyValueCache due to cache dir not exist
@wangmengyan95 wangmengyan95 merged commit da739de into master Sep 18, 2015
@wangmengyan95 wangmengyan95 deleted the wangmengyan.t8376668_NPE_in_ParseKeyValueCache branch September 18, 2015 19:36
@facebook-github-bot
Copy link

@wangmengyan95 updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@wangmengyan95 wangmengyan95

Labels
None yet
Projects
None yet
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.
3 participants
@wangmengyan95 @grantland @facebook-github-bot