@blake.meike rather than creating separate posts, I’ll just start this more general thread for API feedback I come across.
I ran into some additional nullability variances in
setDictionary() all have
@NonNull value parameters in the Java SDK. I noticed these lines of code in the tests violate that contract though. So I checked the Swift and ObjC APIs and the values are nullable there. Similar APIs in
MutableArray are also nullable.
Thanks a million, @jeff.lockhart . This is awesome stuff.
Fixed in 3.1
I noticed the error message thrown here in these tests isn’t accurate:
Non-string or null key in data to be deserialized.
On iOS, the error message is mostly correct:
InvalidType is not a valid type. You may only pass NSNumber, NSString, NSDate, CBLDictionary, CBLArray, Blob, a one-dimensional array or a dictionary whose members are one of the preceding types.
I fixed the error key in this PR, where I fixed some other issues I found with tests. But the shared error message could still use some adjustments to work on both platforms. For example, “Blob” is hard-coded in the message, where it is actually
CBLBlob in the ObjC SDK and is now shown twice in Java, since it’s included in
SUPPORTED_TYPES as well. Also the “array” and “dictionary” language are iOS terms, where in Java the accepted types are
Map, also already in the
@blake.meike there’s a subtle difference in
URLEndpoint, where the Java SDK allows embedding a username only without a password, but the ObjC SDK doesn’t allow embedding either. Which behavior is correct?
Also, not sure if you saw my response here about the incorrect error message. See my reply in this thread above for more details.
I also had this note about the public API. Should this getter not be public in the ObjC SDK?
I fixed a couple test issues I discovered (#1, #2) as well.
I will pass on your suggestion for rewording the error message. Not clear, to me, why “Blob” is hard-coded. Mainwhile, I’ve fixed the key, as you suggested.
WRT the credentials, I believe that the Java API is correct, but I will check.
The constructor for FullTextIndex that takes a List would have to copy it over. I will check on whether we need to add it or not… I’m against it. I will make the getters public if we decide that is correct for the API
I’ve incorporated the changes from your PRs into the 3.1 release. They will probably not go into the 3.0 line
All of this stuff is very much appreciated.