Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

waitForPendingUpdate method #6

Closed
curquiza opened this issue May 4, 2020 · 3 comments · Fixed by #164
Closed

waitForPendingUpdate method #6

curquiza opened this issue May 4, 2020 · 3 comments · Fixed by #164
Assignees

Comments

@curquiza
Copy link
Member

curquiza commented May 4, 2020

Issue fully detailed here.

@ppamorim
Copy link
Contributor

@curquiza Is this feature really needed? The Swift library is 100% async by design.

@bidoubiwa
Copy link
Contributor

bidoubiwa commented Jun 16, 2020

Hey @ppamorim ppamorim!
I think there are two different "asynchronicity".

  1. The asynchronicity of the httprequest. Where you have to wait for MeiliSearch Server to respond
  2. The asynchronicity of MeiliSearch processes. Where you have to wait for your actions to be proccessed by MeiliSearch. (More info here)

This function aims to create a waiting function for the second case, not the first/

When I look into your tests I observe this

   self.client.addOrReplaceDocument(
            UID: uid,
            document: document,
            primaryKey: primaryKey) { result in

            switch result {
            case .success(let update):
                XCTAssertEqual(stubUpdate, update)
                expectation.fulfill()
            case .failure:
                XCTFail("Failed to add or replace Movies document")
            }

        }

        self.wait(for: [expectation], timeout: 1.0)

If I read your code correctly. On the last line of this code block, you wait for addOrReplaceDocument to be fulfilled and you consider it to be fulfilled once MeiliSearch responded with the update information. This is the first case of asynchronicity discussed above.
But this does not mean that the addDocument has been processed by MeiliSearch. It is possibly still in the queue.

The idea with waitForPendingUpdate is that it waits for the second case of asynchronicity discussed above. I'm do not know any swift but let me make a pseudo code with it:

   self.client.addOrReplaceDocument(
            UID: uid,
            document: document,
            primaryKey: primaryKey) { result in

            switch result {
            case .success(let update):
               XCTAssertEqual(stubUpdate, update)
               self.client. waitForPendingUpdate(
                    UID: uid, 
                    updateId: update["updateId"]) { result in 
                    switch result {
                         case .success(let updateInfo):
                               XCTAssertEqual("processed", update.status)
                               expectation.fulfill()
                         case .failure:
                               XCTFail("Failed to add or replace Movies document")
                    }
            }

               }               
            case .failure:
                XCTFail("Failed to add or replace Movies document")
            }

        }

        self.wait(for: [expectation], timeout: 1.0)

This will ensur that the documents have been processed and thus added correctly to MeiliSearch. The user will be able to wait upon any asynchronous MeiliSearch process and not just the MeiliSearch server response. In this case, the user is now sure all of his documents have been added and he can use the search.

Maybe I understood swift wrongly. But in the case I did not, do you feel this has more sense ?

@ppamorim
Copy link
Contributor

@bidoubiwa Oh nice I will implement this! Thank you for the complete explanation.

@curquiza curquiza added the good first issue Good for newcomers label Apr 27, 2021
@bidoubiwa bidoubiwa removed the good first issue Good for newcomers label Sep 1, 2021
@bors bors bot closed this as completed in 9a89a98 Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants