Closed GoogleCodeExporter closed 8 years ago
I would agree that it is rather pointless to make this call with en empty list
of
uids (I fixed my own code too), but this is a result of a more generic problem
with
building the signature when one or more of the param values are null.
Original comment by lauri.h.lehtinen@gmail.com
on 28 Apr 2009 at 9:12
Original comment by david.j....@googlemail.com
on 30 Apr 2009 at 9:48
JUnit test would be great (I'm just trying to get developers to write JUnit
tests at
the moment rather than me taking sample code and writing a JUnit test from it.)
Thanks!
Original comment by david.j....@googlemail.com
on 1 May 2009 at 9:10
Here goes
Original comment by lauri.h.lehtinen@gmail.com
on 1 May 2009 at 3:03
Attachments:
Cool, thanks. The following lines are not necessary (they're meaningless):
Assert.assertTrue( true );
But I'm getting the same failures here as I'm sure you are, so thanks!
Working on this...
Original comment by david.j....@googlemail.com
on 6 May 2009 at 6:33
Line 704 of ExtensibleClient.java contains an assertion:
assert ( !fields.isEmpty() );
So, the original developer built an assumption in that nobody would pass an
empty
list in. I'm not sure that we should actually add any code for this; what's the
behaviour you'd like to see? Perhaps it is worth constructing and throwing a
FacebookException with a decent error message?
Original comment by david.j....@googlemail.com
on 6 May 2009 at 6:41
Well I would expect the client to make the call I'm asking it to make no matter
what
.. be it empty and waste of resources or not.
IMO, there is a bug in the signature generation if a null value finds it's way
there.
When the call to Facebook API is prepared, null values are checked for and
replaced
with empty strings, and that's what I think should happen in the signature
generation
phase.
Original comment by lauri.h.lehtinen@gmail.com
on 6 May 2009 at 6:50
Sure. Would appreciate it if you could create a patch; I'll apply it. Thanks.
Original comment by david.j....@googlemail.com
on 6 May 2009 at 7:04
Did you look at the one that I attached earlier?
Original comment by lauri.h.lehtinen@gmail.com
on 6 May 2009 at 7:05
Sorry, I missed it. I've applied it and checked in the JUnit test with a couple
of
modifications.
All JUnit tests pass, so we can be confident that this small change in a
critical
class hasn't broken anything (much).
Please update your code and ensure you're happy. Thanks. I've also bumped you
to
member status of this project which means you can make code commits if you
like.
Please use with care and run JUnit tests through; feel free to mail me with
patches
you're thinking of committing for review the first couple of times. Enjoy.
Original comment by david.j....@googlemail.com
on 6 May 2009 at 7:30
Thanks, I hope I get a chance contribute smth in the future!
Original comment by lauri.h.lehtinen@gmail.com
on 6 May 2009 at 7:50
Original issue reported on code.google.com by
lauri.h.lehtinen@gmail.com
on 28 Apr 2009 at 9:10Attachments: