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

Throw exception if contentResolver.query() returns null #451

Merged
merged 1 commit into from
Jul 22, 2015
Merged

Throw exception if contentResolver.query() returns null #451

merged 1 commit into from
Jul 22, 2015

Conversation

nikitin-da
Copy link
Collaborator

Also add custom exception.

@nikitin-da nikitin-da added this to the v1.1.0 milestone Jul 18, 2015
@nikitin-da
Copy link
Collaborator Author

Closes #451, also closes #448.
@artem-zinnatullin PTAL =)

@artem-zinnatullin
Copy link
Member

Hm:

com.pushtorefresh.storio.contentresolver.operations.delete.DefaultDeleteResolverTest > performDelete STANDARD_ERROR
    Exception in thread "StorIOContentResolverNotificationsThread" java.lang.NullPointerException
com.pushtorefresh.storio.contentresolver.operations.delete.DefaultDeleteResolverTest > performDelete PASSED
com.pushtorefresh.storio.contentresolver.operations.delete.DeleteResultTest STANDARD_ERROR
        at org.robolectric.shadows.ShadowLooper.getMainLooper(ShadowLooper.java:70)
        at org.robolectric.shadows.ShadowLooper.doLoop(ShadowLooper.java:85)
        at org.robolectric.shadows.ShadowLooper.loop(ShadowLooper.java:76)
        at android.os.Looper.loop(Looper.java)
        at android.os.HandlerThread.run(HandlerThread.java:61)

artem-zinnatullin
artem-zinnatullin reviewed Jul 18, 2015
View reviewed changes
storio-common/src/main/java/com/pushtorefresh/storio/StorIOException.java
/**
* {@inheritDoc}
*/
public StorIOException(String detailMessage) {
Copy link
Member

Choose a reason for hiding this comment

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

@NonNull for `detailMessage.

@artem-zinnatullin
Copy link
Member

LGTM, but comments and test failure

@nikitin-da
Copy link
Collaborator Author

@artem-zinnatullin updated ;)

artem-zinnatullin
artem-zinnatullin reviewed Jul 22, 2015
View reviewed changes
storio-common/src/main/java/com/pushtorefresh/storio/StorIOException.java
/**
* Common {@link RuntimeException} for all errors and exceptions occurred during StorIO operations.
*/
public class StorIOException extends RuntimeException {
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

artem-zinnatullin
artem-zinnatullin reviewed Jul 22, 2015
View reviewed changes
...n/java/com/pushtorefresh/storio/contentresolver/operations/delete/PreparedDeleteByQuery.java
return deleteResolver.performDelete(storIOContentResolver, deleteQuery);
try {
return deleteResolver.performDelete(storIOContentResolver, deleteQuery);
} catch (Throwable throwable) {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, you think we should wrap all Throwables into StorIOException? I'd expect only Exceptions. Imagine if poorly written app used a lot of memory and performing operation in StorIO produces OutOfMemoryError — I don't want to see reports about such problems in our issues tracker 😄

Let's catch only Exception?

@artem-zinnatullin
Copy link
Member

LGTM 👍

The only issue — catch Exception instead of catch Throwable, if you won't have time — I'll do it and then merge your PR.

@nikitin-da
Copy link
Collaborator Author

@artem-zinnatullin Updated again)

@artem-zinnatullin
Copy link
Member

Nice, thanks!

artem-zinnatullin added a commit that referenced this pull request Jul 22, 2015
@artem-zinnatullin
Merge pull request #451 from pushtorefresh/issue-448
dcc61bd 
Throw exception if contentResolver.query() returns null, Add StorIOException
@artem-zinnatullin artem-zinnatullin merged commit dcc61bd into master Jul 22, 2015
@artem-zinnatullin artem-zinnatullin deleted the issue-448 branch July 22, 2015 10:00
@nikitin-da nikitin-da mentioned this pull request Jul 23, 2015
Closed
@artem-zinnatullin artem-zinnatullin mentioned this pull request Jul 27, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@artem-zinnatullin artem-zinnatullin

Labels
bug enhancement || feature
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@nikitin-da @artem-zinnatullin