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