[DataStore] Use prepared statement for query

[raphkim]

Description of changes:

• Fixed a bug where null integer would be deserialized to 0 (same for other primitive types)
• Added nullable array of strings as parameter for SqlCommand
• Added tests to confirm that strings are properly bound

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[lexmakali]

actualPeople

[lexmakali]

expectedPeople, actualPeople

[lexmakali]

Might be better to convert this to a builder in the future.

[raphkim]

I think SqlCommand needs a major refactor but this is my compromise for now

[jamesonwilliams]

Nice!

The SQLiteStorageAdapterInstrumentedTest.java is getting too long. We should move the predicates tests into their own specialized SQLiteStoragePredicatesTest.java, I think.

The other category of test, around model relationships, should probably also be split out into SQLiteStorageRelationshipsTest.java, or something.

[jamesonwilliams]

just end the method here with return ObjectsCompat.equals(this.selectionArgs, that.selectionArgs);

[raphkim]

ugh i'm not a fan of doing this to be honest. We need to either chain all of them into a single return statement or do it the way i did it for purely simplistic design. IIRC java compiler will compile them to be same either way?

[jamesonwilliams]

It would be possible for selectionArgs to get modified external to this class, and then this classes' reference would also see those changes. To be defensive to this, you could do:

this.selectionArgs = Arrays.copyOf(selectionArgs, selectionArgs.length);

https://developer.android.com/reference/java/util/Arrays.html#copyOf(T[],%20int)

[raphkim]

i'll just make it take in a list instead and create a method to return array version, which will use toArray (creating copy)

[jamesonwilliams]

Generally speaking, it's better to have inputs be immutable, and outputs be the result of applying logic onto those immutable inputs.

This args is like a C-style OUT variable, where the input gets mutated:

void init_name(OUT name_t *name) {
    name->first = strdup("Jameson");
    name->last = strdup("Williams");
}

So, I think a solution here would be to have parsePredicate(predicate) return a data structure of some kind. It looks like it (at least) can produce a List<String> selectionArgs, and a String queryString (are these the correct names?)

So maybe it should be like:

    ...
    final class SQLPredicate {
        private final String[] selectionArgs;
        private final String queryString;
        ...
    }

    SQLPredicate parsePredicate(QueryPredicate predicate) {
        ...
    }
    ...

[jamesonwilliams]

resultOfMaliciousQuery,

[jamesonwilliams]

resultAfterMaliciousQuery

[jamesonwilliams]

var name --> final Person jane = Person.builder()...

[jamesonwilliams]

assertEquals(jane, resultAfterMaliciousQuery.next());