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.

Sign up for GitHub

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

Jump to bottom

Actual types passed to generic POJO super classes and to generic collection fields are ignored #1313

Closed
eranl opened this issue Jun 2, 2023 · 6 comments · May be fixed by #1323
Closed

Actual types passed to generic POJO super classes and to generic collection fields are ignored #1313

eranl opened this issue Jun 2, 2023 · 6 comments · May be fixed by #1323
Assignees
@tom-andersen
Labels
api: firestore Issues related to the googleapis/java-firestore API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@eranl
Copy link
Contributor

eranl commented Jun 2, 2023

Given the classes

  private static class BaseBean<T> {
    public T value;
  }

  private static class SubBean extends BaseBean<String> {}

if you try to deserialize into the sub-bean, you get java.lang.IllegalStateException: Could not resolve type T. Same is true for a base bean with a setter that takes a T parameter.

While debugging this, I found that given the classes

  private static class MapBean<T> {
    public Map<String, T> values;
  }

  private static class MapSubBean extends MapBean<String> {}

  private static class MapBeanHolder {
    public MapBean<String> map;
  }

deserializing into MapSubBean or MapBeanHolder, the code errs on the lenient side, and would accept any type of values into the values map.

I have a proposed fix for both issues. Should I post a PR? The collection part of the fix might break existing code, so it may need to be excluded.
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/java-firestore API. label Jun 2, 2023
@tom-andersen tom-andersen self-assigned this Jun 2, 2023
@tom-andersen
Copy link
Contributor

Hello again @eranl!

If a feature is important to you, then submitting a PR will certainly accelerate the process. We are adverse to breaking changes, and will only consider them for major releases. So if we can steer clear of anything that can break code, that will make accepting the PR much easier.

Thanks!

@tom-andersen tom-andersen added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Jun 9, 2023
@eranl
Copy link
Contributor Author

eranl commented Jun 9, 2023

Hi @tom-andersen,

Will do. Actually, in my PR, I will introduce a new @StrictCollectionTypes annotation, and use that to add the fix in a backward-compatible way.

@eranl eranl mentioned this issue Jun 10, 2023
Open
@tom-andersen
Copy link
Contributor

Thanks! I will take a closer look.

@eranl
Copy link
Contributor Author

eranl commented Jul 6, 2023

Hi @tom-andersen, any update?

@eranl
Copy link
Contributor Author

eranl commented Aug 9, 2023

Hi @tom-andersen, would it be more efficient/faster if I report and propose these on the android version first/in parallel?

@eranl eranl mentioned this issue Sep 14, 2023
Open
@tom-andersen
Copy link
Contributor

I have created an internal ticket to track this feature request. b/314353099

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tom-andersen tom-andersen

Labels
api: firestore Issues related to the googleapis/java-firestore API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: handling of superclass generic type parameters eranl/java-firestore
2 participants
@eranl @tom-andersen