724200 - SyncStorageRequest.delete() results in NullPointerException

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P2)

Description

[Marina Samuel [:emtwo]]

addHeader() in delete() should only be called once resource.request has been initialized

Comment 1

[Richard Newman [:rnewman]]

Per IRC discussion:

20:48:57 <@rnewman> err, yeah, that's all completely wrong, isn't it?
20:50:47 <@rnewman> two possibilities
20:50:54 <@rnewman> one: override delete, so you would 
20:51:10 <@rnewman>   foo = new HttpDelete(this.uri);
20:51:16 <@rnewman>   foo.addHeader(...);
20:51:19 <@rnewman>   this.go(foo);
20:52:26 <@rnewman> two: check for delete in SyncStorageResourceDelegate.addHeaders, and add the header there
20:52:40 <@rnewman> I prefer the second option, marina
20:52:49 <@rnewman> good catch :)
20:53:06 < marina> ah, excellent, glad you agree that was weird
20:53:11 < marina> thanks!
20:53:54 <@rnewman> "check for delete": if (request instanceof HttpDelete) { ...
20:54:17 <@rnewman> or
20:54:18 <@rnewman> better
20:54:28 <@rnewman> if (request.getMethod.equalsIgnoreCase("DELETE"))
20:54:43 <@rnewman> there would be an awful edge case bug hiding there


This is what happens when you don't write tests :D

Comment 2

[Richard Newman [:rnewman]]

Fixed in develop:

https://github.com/mozilla-services/android-sync/commit/3133b2aa489c448bac68be7d06ec5e2f590e24db

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/07510613658c