Add support to Encodable types for addDocuments function

[ppamorim]

Summary

• Function addDocuments<T> added. Now it's possible to send objects that are Codable/Encodable and the SDK will automatically encode the values to JSON. A custom encoder can be set by adding encoder in the call.

Additions

• Fixed bug in the ClientTests.swift file

Local tests

• Unit tests passed
• Integration tests passed

Problems

Users are forced to wrap the object to [] when using addDocuments function with objects that are Codable/Encodable. I tried to create a wrapper function for this but Swift doesn't like the direct conversion from T to [T].

[curquiza]

Is this PR ready for review? Because it's still a draft 🙂

[ppamorim]

@curquiza This is waiting for the PR #54.

[curquiza]

bors try

[bors]

try

Merge conflict.

[curquiza]

There is merge conflicts, bors can't do the rebase for you, sorry... Could you rebase your branch, please? 🙂 @ppamorim

[ppamorim]

bors try

[bors]

try

Build failed:

• swift-test

[ppamorim]

bors try

[bors]

try

Build succeeded:

• swift-test

[bidoubiwa]

Could you make a test where we add documents without jsonEncode them? Something like this

func testAddDataAndGetDocuments() {
      let expectation = XCTestExpectation(description: "Add or replace Movies document")

      self.client.addDocuments(
          UID: self.uid,
          documents: movies, //We use movies directly
          primaryKey: nil
      ) { result in
       ...

[curquiza]

@ppamorim Two questions:

• If I understand, we add another addDocuments method additionally to the previously existing one. Does this mean you can use the addDocuments method with or without the encodable support? Maybe I don't understand well, but what the point for the user to use a function that does not handle encodable types? I mean, should we let both of the method addDocuments?

• Should we add this functionality to the updateDocuments method? Not mandatory in this PR, but at least an issue should be created. The #37 is maybe not really accurate. In general, are there any other functions concerned by this user-experience improvement? @bidoubiwa

[ppamorim]

@ppamorim Two questions:

• If I understand, we add another addDocuments method additionally to the previously existing one. Does this mean you can use the addDocuments method with or without the encodable support? Maybe I don't understand well, but what the point for the user to use a function that does not handle encodable types? I mean, should we let both of the method addDocuments?
• Should we add this functionality to the updateDocuments method? Not mandatory in this PR, but at least an issue should be created. The Should not ask user to jsonEncode when using the SDK #37 is maybe not really accurate. In general, are there any other functions concerned by this user-experience improvement? @bidoubiwa

Answers in order:

[0]:

• We added the support of codable types in the addDocuments<T: Codable>(...), this function requires that the user passes an array of T that implements Codable. If you want to directly send the data buffer to the server, you can use the function addDocuments(UID:documents:...) where documents is the type data. This is especially useful if you are adding data without the need to transforming the data to an array of Codable and them passing the data to the JSON encoder, this is an overhead if you already have the JSON data ready for MeiliSearch server.

Ex:

This code gets outdated:

let movie: Movie = Movie(id: 10, title: "test", comment: "test movie")
let documents: Data = try! JSONEncoder().encode([movie])
self.client.addDocuments(
            UID: uid,
            documents: documents,
            primaryKey: nil) { ... }

Now the user is capable of simply sending the values directly:

let movie: Movie = Movie(id: 10, title: "test", comment: "test movie")
self.client.addDocuments(
            UID: uid,
            documents: [movie],
            primaryKey: nil) { ... }

The library will do all the JSONEncoder part automatically.

[1]:

• Yes I agree that this should be implemented too, I will open a PR for this later, is that ok?

[ppamorim]

@curquiza Missed to inform that if the user contains requires a custom date format, it can be passed as an argument by using the parameter JSONEncoder.

[curquiza]

Yes I understand why what you are implementing in this PR is useful, I don't understand why we keep the outdated function in our code base, I mean the function used in this case:

let movie: Movie = Movie(id: 10, title: "test", comment: "test movie")
let documents: Data = try! JSONEncoder().encode([movie])
self.client.addDocuments(
            UID: uid,
            documents: documents,
            primaryKey: nil) { ... }

This version of addDocuments is outdated as you said, can we remove it?

[ppamorim]

This is not outdated because they use may want to pass the data directly without the need of writing a Codable object for it. It will be used in the *** app.

[curquiza]

Let's keep both then.

But, as @bidoubiwa said, could you add a test for that: #53 (review)? -> my bad, I miss-read

Yes I agree that this should be implemented too, I will open a PR for this later, is that ok?

Yes, opening an issue is better!

[curquiza]

LGTM 👌

[curquiza]

bors merge

[bors]

Build succeeded:

• swift-test