Use IllegalStateException to replace invalid JedisDataException (#2393)

* Introduce JedisBatchOperationException

- Pipeline and Transaction related exceptions, which do not come from Redis, are changed to throw JedisBatchOperationException.
- SafeEncoder is changed to throw JedisException.

Based on discussions in #1183

* Update JedisTest.java

* Update JedisTest.java

* Update JedisWithCompleteCredentialsTest.java

* backward compatibility with JedisBatchOperationException

* address review

* remove deprecation

* Use IllegalStateException to replace invalid JedisDataException

* remove unused import

* Use IllegalArgumentException

* Renaming test methods

Co-authored-by: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com>

Author: sazzad16

src/main/java/redis/clients/jedis/BinaryJedis.java

@@ -29,7 +29,6 @@
 import redis.clients.jedis.commands.MultiKeyBinaryCommands;
 import redis.clients.jedis.commands.ProtocolCommand;
 import redis.clients.jedis.exceptions.InvalidURIException;
-import redis.clients.jedis.exceptions.JedisDataException;
 import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.params.*;
 import redis.clients.jedis.resps.*;
@@ -2294,11 +2293,11 @@ public Transaction multi() {
 
   protected void checkIsInMultiOrPipeline() {
     if (client.isInMulti()) {
-      throw new JedisDataException(
+      throw new IllegalStateException(
           "Cannot use Jedis when in Multi. Please use Transaction or reset jedis state.");
     } else if (pipeline != null && pipeline.hasPipelinedResponse()) {
-      throw new JedisDataException(
-          "Cannot use Jedis when in Pipeline. Please use Pipeline or reset jedis state .");
+      throw new IllegalStateException(
+          "Cannot use Jedis when in Pipeline. Please use Pipeline or reset jedis state.");
     }
   }
 

src/main/java/redis/clients/jedis/Pipeline.java

@@ -20,8 +20,8 @@ public List<Object> build(Object data) {
       List<Object> values = new ArrayList<>();
 
       if (list.size() != responses.size()) {
-        throw new JedisDataException("Expected data size " + responses.size() + " but was "
-            + list.size());
+        throw new IllegalStateException(
+            "Expected data size " + responses.size() + " but was " + list.size());
       }
 
       for (int i = 0; i < list.size(); i++) {
@@ -126,14 +126,14 @@ public List<Object> syncAndReturnAll() {
   }
 
   public Response<String> discard() {
-    if (currentMulti == null) throw new JedisDataException("DISCARD without MULTI");
+    if (currentMulti == null) throw new IllegalStateException("DISCARD without MULTI");
     client.discard();
     currentMulti = null;
     return getResponse(BuilderFactory.STRING);
   }
 
   public Response<List<Object>> exec() {
-    if (currentMulti == null) throw new JedisDataException("EXEC without MULTI");
+    if (currentMulti == null) throw new IllegalStateException("EXEC without MULTI");
 
     client.exec();
     Response<List<Object>> response = super.getResponse(currentMulti);
@@ -143,11 +143,10 @@ public Response<List<Object>> exec() {
   }
 
   public Response<String> multi() {
-    if (currentMulti != null) throw new JedisDataException("MULTI calls can not be nested");
+    if (currentMulti != null) throw new IllegalStateException("MULTI calls can not be nested");
 
     client.multi();
-    Response<String> response = getResponse(BuilderFactory.STRING); // Expecting
-    // OK
+    Response<String> response = getResponse(BuilderFactory.STRING); // Expecting OK
     currentMulti = new MultiResponseBuilder();
     return response;
   }

src/main/java/redis/clients/jedis/Response.java

@@ -24,13 +24,12 @@ public void set(Object data) {
   }
 
   public T get() {
-    // if response has dependency response and dependency is not built,
-    // build it first and no more!!
+    // if response has dependency response and dependency is not built, build it first and no more!!
     if (dependency != null && dependency.set && !dependency.built) {
       dependency.build();
     }
     if (!set) {
-      throw new JedisDataException(
+      throw new IllegalStateException(
           "Please close pipeline or multi block before calling this method.");
     }
     if (!built) {

src/main/java/redis/clients/jedis/util/SafeEncoder.java

@@ -5,7 +5,6 @@
 import java.util.List;
 
 import redis.clients.jedis.Protocol;
-import redis.clients.jedis.exceptions.JedisDataException;
 import redis.clients.jedis.exceptions.JedisException;
 
 /**
@@ -28,7 +27,7 @@ public static byte[][] encodeMany(final String... strs) {
   public static byte[] encode(final String str) {
     try {
       if (str == null) {
-        throw new JedisDataException("value sent to redis cannot be null");
+        throw new IllegalArgumentException("null value cannot be sent to redis");
       }
       return str.getBytes(Protocol.CHARSET);
     } catch (UnsupportedEncodingException e) {

src/test/java/redis/clients/jedis/tests/JedisTest.java

@@ -25,7 +25,6 @@
 import redis.clients.jedis.Protocol;
 import redis.clients.jedis.exceptions.InvalidURIException;
 import redis.clients.jedis.exceptions.JedisConnectionException;
-import redis.clients.jedis.exceptions.JedisDataException;
 import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.tests.commands.JedisCommandTestBase;
 import redis.clients.jedis.util.SafeEncoder;
@@ -128,7 +127,7 @@ public void infiniteTimeout() throws Exception {
     }
   }
 
-  @Test(expected = JedisDataException.class)
+  @Test(expected = IllegalArgumentException.class)
   public void failWhenSendingNullValues() {
     jedis.set("foo", null);
   }
@@ -248,4 +247,4 @@ public void checkDisconnectOnQuit() {
     assertFalse(jedis.isConnected());
   }
 
-}
+}

src/test/java/redis/clients/jedis/tests/JedisWithCompleteCredentialsTest.java

@@ -142,4 +142,4 @@ public void allowUrlWithNoDBAndNoPassword() {
     }
   }
 
-}
+}

src/test/java/redis/clients/jedis/tests/PipeliningTest.java

@@ -183,7 +183,7 @@ public void pipelineResponseWithoutData() {
     assertNull(score.get());
   }
 
-  @Test(expected = JedisDataException.class)
+  @Test(expected = IllegalStateException.class)
   public void pipelineResponseWithinPipeline() {
     jedis.set("string", "foo");
 
@@ -351,28 +351,28 @@ public void multiUnwatch() {
     assertEquals(expect, pipe.syncAndReturnAll());
   }
 
-  @Test(expected = JedisDataException.class)
-  public void pipelineExecShoudThrowJedisDataExceptionWhenNotInMulti() {
+  @Test(expected = IllegalStateException.class)
+  public void pipelineExecWhenNotInMulti() {
     Pipeline pipeline = jedis.pipelined();
     pipeline.exec();
   }
 
-  @Test(expected = JedisDataException.class)
-  public void pipelineDiscardShoudThrowJedisDataExceptionWhenNotInMulti() {
+  @Test(expected = IllegalStateException.class)
+  public void pipelineDiscardWhenNotInMulti() {
     Pipeline pipeline = jedis.pipelined();
     pipeline.discard();
   }
 
-  @Test(expected = JedisDataException.class)
-  public void pipelineMultiShoudThrowJedisDataExceptionWhenAlreadyInMulti() {
+  @Test(expected = IllegalStateException.class)
+  public void pipelineMultiWhenAlreadyInMulti() {
     Pipeline pipeline = jedis.pipelined();
     pipeline.multi();
     pipeline.set("foo", "3");
     pipeline.multi();
   }
 
-  @Test(expected = JedisDataException.class)
-  public void testJedisThowExceptionWhenInPipeline() {
+  @Test(expected = IllegalStateException.class)
+  public void testJedisThrowExceptionWhenInPipeline() {
     Pipeline pipeline = jedis.pipelined();
     pipeline.set("foo", "3");
     jedis.get("somekey");
@@ -680,7 +680,7 @@ public void testCloseableWithMulti() throws IOException {
     try {
       pipeline.exec();
       fail("close should discard transaction");
-    } catch (JedisDataException e) {
+    } catch (IllegalStateException e) {
       assertTrue(e.getMessage().contains("EXEC without MULTI"));
       // pass
     }

src/test/java/redis/clients/jedis/tests/ShardedJedisPipelineTest.java

@@ -23,7 +23,6 @@
 import redis.clients.jedis.ShardedJedis;
 import redis.clients.jedis.ShardedJedisPipeline;
 import redis.clients.jedis.Tuple;
-import redis.clients.jedis.exceptions.JedisDataException;
 
 public class ShardedJedisPipelineTest {
 
@@ -109,7 +108,7 @@ public void pipelineResponse() {
     assertEquals(1, zrangeWithScores.get().size());
   }
 
-  @Test(expected = JedisDataException.class)
+  @Test(expected = IllegalStateException.class)
   public void pipelineResponseWithinPipeline() {
     jedis.set("string", "foo");
 

src/test/java/redis/clients/jedis/tests/commands/TransactionCommandsTest.java

@@ -156,7 +156,7 @@ public void unwatch() {
     assertEquals("OK", resp.get(0));
   }
 
-  @Test(expected = JedisDataException.class)
+  @Test(expected = IllegalStateException.class)
   public void validateWhenInMulti() {
     jedis.multi();
     jedis.ping();
@@ -215,7 +215,7 @@ public void transactionResponseBinary() {
     assertArrayEquals("foo".getBytes(), set.get());
   }
 
-  @Test(expected = JedisDataException.class)
+  @Test(expected = IllegalStateException.class)
   public void transactionResponseWithinPipeline() {
     jedis.set("string", "foo");