Skip to content
This repository was archived by the owner on Dec 20, 2025. It is now read-only.

fix(retrofit2): fix retrofit2 issues#1875

Merged
mergify[bot] merged 3 commits intospinnaker:masterfrom
kirangodishala:fix-retrofit2-querymap-issue
Feb 27, 2025
Merged

fix(retrofit2): fix retrofit2 issues#1875
mergify[bot] merged 3 commits intospinnaker:masterfrom
kirangodishala:fix-retrofit2-querymap-issue

Conversation

@kirangodishala
Copy link
Contributor

@kirangodishala kirangodishala commented Feb 27, 2025

Addressed the following two issues which occurred due to restrictions in retrofit2 :

  • Map must include generic types (e.g., Map<String, String>)
  • A @path parameter must not come after a @query

…ryMap is used without generics:

`java.lang.IllegalArgumentException: Map must include generic types (e.g., Map<String, String>)`
…error:

`java.lang.IllegalArgumentException: Map must include generic types (e.g., Map<String, String>)`
}

List<Map> search(String provider, String query, String region, String account, Integer count, Map<String, Object> additionalFilters, String selectorKey) {
List<Map> search(String provider, String query, String region, String account, Integer count, Map<String, String> additionalFilters, String selectorKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently the only call to this is from ImageController.findImages and the value in the Map it uses is a String, so this seems safe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I merged this too soon...I don't think it's sufficient. We're missing more test coverage and groovy isn't helping. That code in ImageController is gonna fail with

'search' in 'com. netflix. spinnaker. gate. services. ImageService' cannot be applied to '(java. lang. String, java. lang. String, java. lang. String, java. lang. String, java. lang. Integer, java. util. Map<java. lang. Object,java. lang. Object>, java. lang. String)' 

I'm looking in to a test/fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently #1877 isn't required, but it makes me feel better.

@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Feb 27, 2025
@mergify mergify bot added the auto merged label Feb 27, 2025
@mergify mergify bot merged commit bdf8922 into spinnaker:master Feb 27, 2025
5 checks passed
@dbyron-sf
Copy link
Contributor

@Mergifyio backport release-1.37.x

@mergify
Copy link
Contributor

mergify bot commented Feb 27, 2025

backport release-1.37.x

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Feb 27, 2025
* test(retrofit2): add test to demonstrate the following error when QueryMap is used without generics:

`java.lang.IllegalArgumentException: Map must include generic types (e.g., Map<String, String>)`

* fix(retrofit2): fix non-generics QueryMap which causes the following error:

`java.lang.IllegalArgumentException: Map must include generic types (e.g., Map<String, String>)`

* fix(retrofit2): fix `A @path parameter must not come after a @Query` error

(cherry picked from commit bdf8922)
mergify bot added a commit that referenced this pull request Feb 27, 2025
* test(retrofit2): add test to demonstrate the following error when QueryMap is used without generics:

`java.lang.IllegalArgumentException: Map must include generic types (e.g., Map<String, String>)`

* fix(retrofit2): fix non-generics QueryMap which causes the following error:

`java.lang.IllegalArgumentException: Map must include generic types (e.g., Map<String, String>)`

* fix(retrofit2): fix `A @path parameter must not come after a @Query` error

(cherry picked from commit bdf8922)

Co-authored-by: Kiran Godishala <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants