Skip to content
This repository was archived by the owner on Sep 28, 2022. It is now read-only.

Add collection:truncate#80

Merged
scottinet merged 8 commits into
3-devfrom
KZL-1462-collection-truncate
Apr 2, 2020
Merged

Add collection:truncate#80
scottinet merged 8 commits into
3-devfrom
KZL-1462-collection-truncate

Conversation

@y-abs
Copy link
Copy Markdown
Contributor

@y-abs y-abs commented Mar 26, 2020

What does this PR do ?

This PR implements the collection:truncate method with its unit tests.

How should this be manually tested?

Clone this branch and run unit tests
./gradlew test

When it succeed, compile it

./gradlew jar

Initiate another java project by adding the compiled SDK as a dependency.

Then, run Kuzzle, create an index nyc-open-data, a yellow-taxi collection with several documents in it.
Finally, run this code

import io.kuzzle.sdk.*;
import io.kuzzle.sdk.Options.KuzzleOptions;
import io.kuzzle.sdk.Options.Protocol.WebSocketOptions;
import io.kuzzle.sdk.Protocol.WebSocket;

import java.util.concurrent.ConcurrentHashMap;

public class truncateCollection {
    private static Kuzzle kuzzle;

    public static void main(String[] args) {
        WebSocketOptions opts = new WebSocketOptions();
        opts.setAutoReconnect(true).setConnectionTimeout(42000);

        try {
            WebSocket ws = new WebSocket("localhost", opts);

            kuzzle = new Kuzzle(ws, (KuzzleOptions) null);

            kuzzle.connect();

            kuzzle.getCollectionController().truncate("nyc-open-data", "yellow-taxi").get();
            
        }  catch (Exception e) {
            e.printStackTrace();
        }

        kuzzle.disconnect();
    }
};

@y-abs y-abs self-assigned this Mar 26, 2020
Comment thread doc/3/controllers/collection/truncate/index.md Outdated

## Returns

A `ConcurrentHashMap` which has an `acknowledged` Boolean.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, you can return nothing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but it's useless IMHO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API returns an acknowledgement. This is a way of saying: "the query has been accepted and it ran fine, but there is no result to return".

So, in a SDK, this is a same than a function returning normally. Bothering with creating a ConcurrentHashMap just to return an acknowledgement has no point.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And to me, this warrants a "requested change", so I'm going to block this PR over this issue.

y-abs and others added 2 commits March 27, 2020 12:02
Co-Authored-By: Adrien Maret <amaret93@gmail.com>
Co-Authored-By: Adrien Maret <amaret93@gmail.com>
Copy link
Copy Markdown
Contributor

@scottinet scottinet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API acknowledgements are not return values.

@y-abs y-abs requested a review from scottinet March 27, 2020 13:34
import io.kuzzle.sdk.Kuzzle;

import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency seems unused

@y-abs y-abs requested a review from scottinet April 1, 2020 07:15
@scottinet scottinet merged commit cbec142 into 3-dev Apr 2, 2020
@scottinet scottinet deleted the KZL-1462-collection-truncate branch April 2, 2020 11:13
@jenow jenow mentioned this pull request May 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants