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.

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

@QueryMap not working with POJOs as specified in documentation #691

Closed
doublemc opened this issue Apr 27, 2018 · 9 comments
Closed

@QueryMap not working with POJOs as specified in documentation #691

doublemc opened this issue Apr 27, 2018 · 9 comments
Labels
needs info Information is either missing or incomplete.

Comments

@doublemc
Copy link

doublemc commented Apr 27, 2018

According to: https://github.com/OpenFeign/feign#dynamic-query-parameters it should be possible to write:
@RequestLine("GET /api/v1/historicalTrades") fun getHistoricalTrades(@QueryMap orderBookQuery: OrderBookQuery): List<TradeEntry>

where OrderBookQuery looks like that:
data class OrderBookQuery(val symbol: String, val limit: Int? = 100)

Feign should generate: /api/v1/historicalTrades?symbol={symobl}&limit={limit} but I'm getting:

Exception in thread "main" java.lang.IllegalStateException: QueryMap parameter must be a Map: class com.binance.api.feign.marketdata.model.OrderBookQuery
	at feign.Util.checkState(Util.java:128)
	at feign.Contract$BaseContract.parseAndValidateMetadata(Contract.java:126)
	at feign.Contract$BaseContract.parseAndValidatateMetadata(Contract.java:64)
	at feign.ReflectiveFeign$ParseHandlersByName.apply(ReflectiveFeign.java:146)
	at feign.ReflectiveFeign.newInstance(ReflectiveFeign.java:53)
	at feign.Feign$Builder.target(Feign.java:198)
	at feign.Feign$Builder.target(Feign.java:194)

Here is my config and usage that produces above exception:

val marketDataRestClient =
        Feign.builder().encoder(GsonEncoder())
            .decoder(GsonDecoder())
            .logger(Logger.ErrorLogger())
            .logLevel(Logger.Level.FULL)
            .target(MarketDataRestClient::class.java, "https://api.binance.com")
    val s = marketDataRestClient.getHistoricalTrades(OrderBookQuery("ETHBTC"))

Link to my SO if someone wants to get them free karma points ;) : https://stackoverflow.com/questions/50044283/feign-querymap-usage-with-pojo

@kdavisk6
Copy link
Member

I don't believe this feature is available in the released versions. It's been accepted to master and will be part of 9.7. If you want to use this now, you will need to build the library from master.

@doublemc
Copy link
Author

DO you know when 9.7 is going live?

@velo
Copy link
Member

velo commented Apr 29, 2018 via email

@kdavisk6
Copy link
Member

kdavisk6 commented May 7, 2018

9.7 was released.

@kdavisk6
Copy link
Member

@doublemc Are you still experiencing this issue with 9.7?

@kdavisk6 kdavisk6 added the needs info Information is either missing or incomplete. label Jul 26, 2018
@3p5i10n
Copy link

3p5i10n commented Aug 30, 2018

Bug is still present, neither 9.7 nor 10.0.1 solve the Issue.

java.lang.IllegalStateException: QueryMap parameter must be a Map: class org.foo.QueryParameters

No QueryMapEncoder is used, as mentioned in the docs.

@RequestLine("GET /v1/foo/") public Foo getFoo(@QueryMap QueryParameters queryParameters);

@rage-shadowman
Copy link
Contributor

Do you have a small failing test case example?

@kdavisk6
Copy link
Member

I'm looking at the latest code and line 126 in Contract does not call checkState anymore, it does a direct check for the map:

if (data.queryMapIndex() != null) {
if (Map.class.isAssignableFrom(parameterTypes[data.queryMapIndex()])) {
checkMapKeys("QueryMap", genericParameterTypes[data.queryMapIndex()]);
}

Are you sure you are using the right feign jar file and version?

@3p5i10n
Copy link

3p5i10n commented Oct 25, 2018

While trying to reproduce it again in a separate sample project, i was not able to.
Now 9.7 and 10.0.1 are working fine ...have to check my base project again.

Sorry for the inconvenience and the late response, been busy the last weeks.

@kdavisk6 kdavisk6 closed this as completed Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info Information is either missing or incomplete.
Projects
None yet
Development

No branches or pull requests

5 participants