@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 MutableDictionary
. setBlob()
, setDate()
, setArray()
, and 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
1 Like
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 List
and Map
, also already in the SUPPORTED_TYPES
.
@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.
@blake.meike I believe the Database.exists()
directory
parameter should be nullable, similar to Database.delete()
. It’s nullable in ObjC and Swift.
There is definitely something wrong, there. As it stands, though, it is delete
that is broken. If you pass a null directory to it, it will error in the null check in exists
.
Lemme see what the other platforms do if that arg is null.
Ok. Took me a minute. This change will cause a warning in Kotlin, so I need to verify that it is ok with the keeper of the API. I presume it is, though. Without objection, it will be in the next 3.1 build.
Thanks again.
1 Like
Thanks. I noticed Database.copy()
similarly has a nullable config
parameter in the iOS SDK, using the default config if null.
Oh lordy. Probably Jens….
Will fix.
Thanks
-blake
@blake.meike I noticed this dictionary test is creating a MutableDocument
instead of a MutableDictionary
. Should be: new MutableDictionary(readJSONResource("array.json"))
.
6 dozen of one; half of the other.
It is inconsistent, though. Will change. Thanks.
@blake.meike working on adding the enterprise APIs to my KMP bindings, I noticed a nullability conflict in MessageEndpoint.target
. The constructor has it @Nullable
, noting it as optional, but the getter is @NonNull
.
Yep. Will fix. I am surprised that none of my static analysis tools are catching these…
@blake.meike in the EE API, the ServerCertificateVerificationMode
enum doesn’t seem to be used anywhere. Maybe it was intended for what ReplicatorConfiguration.isAcceptOnlySelfSignedServerCertificate
now does. Seems to have been added in v2.8.0, but doesn’t seem to be used in that version either. Is this enum necessary for anything?
@blake.meike MessageEndpoint.target
is @Nullable
in the constructor, but the getter is marked @NonNull
. They’re both nullable in the Swift API.
I think you must have mentioned this before… because both are fixed and there’s no way I would have caught 'em
The MessageEndpoint field, constructor parameter and the getter are all nullable in 3.1.x
The verification mode enum, intended, almost certainly, as you imagined, is gone
That’s great they’re fixed already! I should have checked the latest code state.
I’ve been using the Swift SDK tests to implement EE API tests, since EE tests aren’t in the Java CE source code. I also added the subscript/fragment APIs that are in the Swift, Objective-C, and .NET SDK APIs, but not Java, since it doesn’t have operator support. But Kotlin does!