> On Jan. 26, 2017, 2:01 p.m., Jared Stewart wrote:
> > Can you help me understand what the introspectAndConvert method is trying
> > to do? Its use of generics seems very strange to me and it has many
> > unchecked casts which could fail at runtime.
>
> Kevin Duling wrote:
> The idea behind it is to convert a String in to an object map, parsing
> out the key/value pairs. The NPE I was encountering is because the method is
> always expecting a map at this point. Even when passing in a single object,
> the call stack shows it's trying to force the object to be a map. However, a
> function may be written to not require any arguments, as in my NoArgFunction
> test object. That edge case is what created the NPE.
>
> Jared Stewart wrote:
> Thanks for the explanation. I think it would be cleaner to eliminate the
> generic type `<T>` from this method, change the method signature to `
> protected Object introspectAndConvert(final Object value) {`, and to remove
> the `(T)` casts.
That has further reaching reprocussions and I don't know that we have enough
test cases to cover it. I've only added a null check to a pre-existing method
to handle an edge case. A refactor such as that should be a separate ticket.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55781/#review163180
-----------------------------------------------------------
On Jan. 25, 2017, 4:35 p.m., Kevin Duling wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55781/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 4:35 p.m.)
>
>
> Review request for geode, Jinmei Liao, Jared Stewart, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Protection against an empty JSON object being sent as no-arguments in to a
> function within the REST controller.
>
>
> Diffs
> -----
>
>
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
> fc2da8b5b164da2f93472ad2dc1dc3a71cf700fd
>
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/NoArgumentFunction.java
> PRE-CREATION
>
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsQueryAndFEJUnitTest.java
> f05411049e3342c3ff84cefc329520802370f039
>
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
> b9d2bf4dec5b8d091207b6a98ff30d11ba7cc822
>
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
> 2169cb28cd80d675764ff185ca1ff7c6dbc2b525
>
> Diff: https://reviews.apache.org/r/55781/diff/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Kevin Duling
>
>
