fix: add support to deserialize to custom Lists and Maps

[suraj-qlogic]

Fixes #318

[codecov]

Codecov Report

Merging #337 into master will decrease coverage by 0.08%.
The diff coverage is 55.55%.

@@ Coverage Diff @@ ## master #337 +/- ## ============================================ - Coverage 72.76% 72.68% -0.09%  - Complexity 1045 1046 +1  ============================================ Files 64 64 Lines 5512 5528 +16 Branches 681 685 +4 ============================================ + Hits 4011 4018 +7  - Misses 1289 1297 +8  - Partials 212 213 +1 

Impacted Files	Coverage Δ	Complexity Δ
.../com/google/cloud/firestore/CustomClassMapper.java	76.84% <55.55%> (-0.73%)	111.00 <0.00> (+2.00)	⬇️
...in/java/com/google/cloud/firestore/BulkWriter.java	71.02% <0.00%> (-0.47%)	28.00% <0.00%> (-1.00%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44c806c...8843b5a. Read the comment docs.

[schmidt-sebastian]

I think this looks good, but it seems like (despite what is mentioned in #318) the same problem exists in Map (we always return a HashMap). Should we fix both instances?

[schmidt-sebastian]

Just FYI: While uncommon, I think this method would now throw if called on an type that represents an subinterface of List, e.g.:

interace MyList extends List { } class Pojo { MyList data; } 

I am however not 100% certain that is the case, but based on my reading of https://stackoverflow.com/questions/36564041/getdeclaredconstructor-on-an-interface, this would throw an NoSuchMethodException, whereas before it would just create an ArrayList (albeit of the wrong type). I don't know if there is a solution that doesn't involve creating a list of the wrong type, and if there isn't, we can probably leave this logic as is.

[suraj-qlogic]

@schmidt-sebastian ,Yes you are right.Whenever I try to create an instance of interface it's produce NoSuchMethodException. Using reflection api won't provide support for creating an instance of interface.So alternative i think to produce deserialization error.

try { result =(rawType == List.class) ? new ArrayList<>(list.size()) : (List<Object>) rawType.getDeclaredConstructor().newInstance(); }catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) { throw deserializeError(context.errorPath,String.format("Deserializing values to %s interface is not supported : %s", rawType.getSimpleName(),e.toString())); } 

[schmidt-sebastian]

@schmidt-sebastian ,Yes you are right.Whenever I try to create an instance of interface it's produce NoSuchMethodException. Using reflection api won't provide support for creating an instance of interface.So alternative i think to produce deserialization error.

That error message LGTM. Do you want to update the PR and also add Map support?

[suraj-qlogic]

@schmidt-sebastian ,Thanks i will update this ASAP.

[dmitry-fa]

a nit:

"Deserializing values to %s interface is not supported : %s"

rawType is not necessary to be an interface to cause a deserialization problem, it could be an abstract class, a class without the default constructor, or with the private default constructor.

I would rephrase: "Unable to deserialize to the %s type: %s"

[schmidt-sebastian]

Suggested change

	"Unable to deserialize to the %s type: %s",
	"Unable to deserialize to %s: %s",

[suraj-qlogic]

Done

[schmidt-sebastian]

Suggested change

	"Unable to deserialize to the %s type: %s", rawType.getSimpleName(), e.toString()));
	"Unable to deserialize to %s: %s",

[suraj-qlogic]

Done