isapego commented on a change in pull request #8174:
URL: https://github.com/apache/ignite/pull/8174#discussion_r492641241
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -117,6 +178,25 @@
*/
public void putAll(Map<? extends K, ? extends V> map) throws
ClientException;
+ /**
+ * Copies all of the entries from the specified map to the {@link
ClientCache}.
+ * <p>
+ * The effect of this call is equivalent to that of calling
+ * {@link #put(Object, Object) put(k, v)} on this cache once for each
mapping
+ * from key <tt>k</tt> to value <tt>v</tt> in the specified map.
+ * <p>
+ * The order in which the individual puts occur is undefined.
+ * <p>
+ * The behavior of this operation is undefined if entries in the cache
+ * corresponding to entries in the map are modified or removed while this
+ * operation is in progress. or if map is modified while the operation is
in
+ * progress.
+ * <p>
+ *
+ * @param map Mappings to be stored in this cache.
Review comment:
`@return` tag?
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -77,6 +111,12 @@
*/
public ClientCacheConfiguration getConfiguration() throws ClientException;
+ /**
+ * Gets the cache configuration asynchronously.
+ * @return The cache configuration.
Review comment:
Maybe,
> The cache configuration **future**
Also, in other docs.
##########
File path: modules/core/src/main/java/org/apache/ignite/client/IgniteClient.java
##########
@@ -57,25 +72,50 @@
*/
public Collection<String> cacheNames() throws ClientException;
+ /**
+ * @return Collection of names of currently available caches or an empty
collection if no caches are available.
+ */
+ public IgniteClientFuture<Collection<String>> cacheNamesAsync() throws
ClientException;
+
/**
* Destroy cache.
*/
public void destroyCache(String name) throws ClientException;
+ /**
+ * Destroy cache.
+ */
+ public IgniteClientFuture<Void> destroyCacheAsync(String name) throws
ClientException;
+
/**
* Create cache.
*
* @param name Cache name.
*/
public <K, V> ClientCache<K, V> createCache(String name) throws
ClientException;
+ /**
+ * Create cache.
Review comment:
Probably, it worth to mention that the default configuration is going to
be used.
##########
File path:
modules/core/src/main/java/org/apache/ignite/client/ClientCompute.java
##########
@@ -55,9 +55,23 @@
* @return A Future representing pending completion of the task.
* @throws ClientException If task failed.
* @see ComputeTask for information about task execution.
+ * @deprecated Use {@link ClientCompute#executeAsync2(String, Object)}
Review comment:
Maybe needs an explanation why is it deprecated.
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -88,6 +128,17 @@
*/
public int size(CachePeekMode... peekModes) throws ClientException;
+ /**
+ * Gets the number of all entries cached across all nodes. By default, if
{@code peekModes} value isn't provided,
+ * only size of primary copies across all nodes will be returned. This
behavior is identical to calling
+ * this method with {@link CachePeekMode#PRIMARY} peek mode.
+ * <p>
+ * NOTE: this operation is distributed and will query all participating
nodes for their cache sizes.
+ *
+ * @param peekModes Optional peek modes. If not provided, then total cache
size is returned.
Review comment:
Add `@return` javadoc?
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -117,6 +178,25 @@
*/
public void putAll(Map<? extends K, ? extends V> map) throws
ClientException;
+ /**
+ * Copies all of the entries from the specified map to the {@link
ClientCache}.
+ * <p>
+ * The effect of this call is equivalent to that of calling
+ * {@link #put(Object, Object) put(k, v)} on this cache once for each
mapping
+ * from key <tt>k</tt> to value <tt>v</tt> in the specified map.
+ * <p>
+ * The order in which the individual puts occur is undefined.
+ * <p>
+ * The behavior of this operation is undefined if entries in the cache
+ * corresponding to entries in the map are modified or removed while this
+ * operation is in progress. or if map is modified while the operation is
in
Review comment:
> ...progress. or if...
Remove dot or replace `or` with `Or`
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -293,11 +529,36 @@
*/
public boolean putIfAbsent(K key, V val) throws ClientException;
+ /**
+ * Atomically associates the specified key with the given value if it is
not already associated with a value.
+ * <p>
+ * This is equivalent to:
+ * <pre><code>
+ * if (!cache.containsKey(key)) {}
+ * cache.put(key, value);
+ * return true;
+ * } else {
+ * return false;
+ * }
+ * </code></pre>
+ * except that the action is performed atomically.
+ *
+ * @param key Key with which the specified value is to be associated.
+ * @param val Value to be associated with the specified key.
+ * @return <tt>true</tt> if a value was set.
+ */
+ public IgniteClientFuture<Boolean> putIfAbsentAsync(K key, V val) throws
ClientException;
+
/**
* Clears the contents of the cache.
*/
public void clear() throws ClientException;
+ /**
+ * Clears the contents of the cache.
Review comment:
Probably, needs a `@return` tag and the information about how this is
different from `removeAllAsync`
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -176,6 +297,24 @@
*/
public boolean remove(K key) throws ClientException;
+ /**
+ * Removes the mapping for a key from this cache if it is present.
+ * <p>
+ * More formally, if this cache contains a mapping from key <tt>k</tt> to
value <tt>v</tt> such that
+ * <code>(key==null ? k==null : key.equals(k))</code>, that mapping is
removed.
+ * (The cache can contain at most one such mapping.)
+ *
+ * <p>Returns <tt>true</tt> if this cache previously associated the key,
or <tt>false</tt> if the cache
+ * contained no mapping for the key.
+ * <p>
+ * The cache will not contain a mapping for the specified key once the
Review comment:
Minor: Looks like the line break is at the different position every time
:)
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -205,13 +364,29 @@
*/
public void removeAll(Set<? extends K> keys) throws ClientException;
+ /**
+ * Removes entries for the specified keys.
+ * <p>
+ * The order in which the individual entries are removed is undefined.
+ *
+ * @param keys The keys to remove.
+ */
+ public IgniteClientFuture<Void> removeAllAsync(Set<? extends K> keys)
throws ClientException;
+
/**
* Removes all of the mappings from this cache.
* <p>
* The order that the individual entries are removed is undefined.
*/
public void removeAll() throws ClientException;
+ /**
+ * Removes all of the mappings from this cache.
+ * <p>
+ * The order that the individual entries are removed is undefined.
Review comment:
Maybe it needs to mention if the operation is atomic
##########
File path: modules/core/src/main/java/org/apache/ignite/client/ClientCache.java
##########
@@ -138,6 +218,27 @@
*/
public boolean replace(K key, V oldVal, V newVal) throws ClientException;
+ /**
+ * Atomically replaces the entry for a key only if currently mapped to a
given value.
+ * <p>
+ * This is equivalent to:
+ * <pre><code>
+ * if (cache.containsKey(key) && equals(cache.get(key), oldValue))
{
+ * cache.put(key, newValue);
+ * return true;
+ * } else {
+ * return false;
+ * }
+ * </code></pre>
+ * except that the action is performed atomically.
Review comment:
Here in below.
The passage about atomicity (which is an important note) is placed at the
end of the passage and the one can easily miss it IMO. May be it makes sense to
reformat the first sentence to something like:
>This is equivalent to performing the following operations as a single
atomic action:
Not a mandatory change request though, just a comment
##########
File path:
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java
##########
@@ -115,6 +118,9 @@
/** Reconnect throttling retries. See {@code reconnectThrottlingPeriod}. */
private int reconnectThrottlingRetries = 3;
+ /** Executor for async operations continuations. */
+ private Executor asyncContinuationExecutor;
Review comment:
Is it `null` by default? Does it mean, user will get an exception in
runtime if it uses continuation with no executor set?
Maybe, we should set some implementation here as a default value? I'm pretty
much sure, it is going to be fine for the most users, at least at the beginning
of the development.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]