timoninmaxim commented on code in PR #10318:
URL: https://github.com/apache/ignite/pull/10318#discussion_r1034384977


##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/transactions/AtomicOperationsInTxTest.java:
##########
@@ -58,122 +61,186 @@ public class AtomicOperationsInTxTest extends 
GridCommonAbstractTest {
     /** {@inheritDoc} */
     @Override protected void beforeTestsStarted() throws Exception {
         super.beforeTestsStarted();
+    }
 
+    /** {@inheritDoc} */
+    @Override protected void beforeTest() throws Exception {
+        super.beforeTest();
         startGrid(0);
     }
 
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        super.afterTest();
+        stopGrid(0);
+    }
+
     /**
+     * Tests whether enabling atomic cache operations within transactions is 
allowed.
+     * Since 2.15.0 the default behaviour is set to {@code false}.
      * @throws Exception If failed.
      */
     @Test
+    @WithSystemProperty(key = IGNITE_ALLOW_ATOMIC_OPS_IN_TX, value = "false")
     public void testEnablingAtomicOperationDuringTransaction() throws 
Exception {
         GridTestUtils.assertThrows(log, (Callable<IgniteCache>)() -> {
             try (Transaction tx = grid(0).transactions().txStart()) {
                 return 
grid(0).cache(DEFAULT_CACHE_NAME).withAllowAtomicOpsInTx();
             }
         },
             IllegalStateException.class,
-            "Enabling atomic operations during active transaction is not 
allowed."
+            "Enabling atomic operations during active transaction is not 
allowed. Enable atomic operations before " +
+                    "transaction start."
         );
     }
 
     /**
+     * Tests whether allowing atomic cache operations before transaction 
starts does not cause exceptions.
+     * Since 2.15.0 the default behaviour is set to {@code false}.
+     * @throws Exception If failed.
+     */
+    @Test
+    @WithSystemProperty(key = IGNITE_ALLOW_ATOMIC_OPS_IN_TX, value = "true")
+    public void testAllowAtomicOperationsBeforeTransactionStarts() throws 
Exception {
+        try (Transaction tx = grid(0).transactions().txStart()) {
+            grid(0).cache(DEFAULT_CACHE_NAME).withAllowAtomicOpsInTx();
+        }
+    }
+
+    /**
+     * Tests that operations with atomic cache within transactions are allowed 
if the system property
+     * {@link IgniteSystemProperties#IGNITE_ALLOW_ATOMIC_OPS_IN_TX 
IGNITE_ALLOW_ATOMIC_OPS_IN_TX} is not changed from
+     * the default {@code false} and the user explicitly allows atomic 
operations in
+     * transactions via {@link IgniteCache#withAllowAtomicOpsInTx() 
withAllowAtomicOpsInTx()} before transactions start.
+     * Since 2.15.0 the default behaviour is set to {@code false}.
+     * @throws Exception If failed.
+     */
+    @Test
+    @WithSystemProperty(key = IGNITE_ALLOW_ATOMIC_OPS_IN_TX, value = "false")
+    public void testSetOfNonAtomicOperationsWithinTransactions() throws 
Exception {
+        checkOperations(true);
+    }
+
+    /**
+     * Tests that operations with atomic cache within transactions are allowed 
if the system property
+     * {@link IgniteSystemProperties#IGNITE_ALLOW_ATOMIC_OPS_IN_TX 
IGNITE_ALLOW_ATOMIC_OPS_IN_TX} is changed to
+     * {@code true} before transaction start and the user explicitly allows 
atomic operations in transactions via
+     * {@link IgniteCache#withAllowAtomicOpsInTx() withAllowAtomicOpsInTx()} 
before transactions start.
+     * Since 2.15.0 the default behaviour is set to {@code false}.
      * @throws Exception If failed.
      */
     @Test
-    public void testAllowedAtomicOperations() throws Exception {
+    @WithSystemProperty(key = IGNITE_ALLOW_ATOMIC_OPS_IN_TX, value = "true")
+    public void testSetOfAtomicOperationsWithinTransactions() throws Exception 
{
         checkOperations(true);
     }
 
     /**
+     * Tests that atomic cache operations within transactions are forbidden if 
system property
+     * {@link IgniteSystemProperties#IGNITE_ALLOW_ATOMIC_OPS_IN_TX 
IGNITE_ALLOW_ATOMIC_OPS_IN_TX}
+     * is changed to {@code true} and the user does not explicitly allow 
atomic operations in transactions via
+     * {@link IgniteCache#withAllowAtomicOpsInTx() withAllowAtomicOpsInTx()} 
before transactions start.
+     * Since 2.15.0 the default behaviour is set to {@code false}.
      * @throws Exception If failed.
      */
     @Test
-    public void testNotAllowedAtomicOperations() throws Exception {
+    @WithSystemProperty(key = IGNITE_ALLOW_ATOMIC_OPS_IN_TX, value = "false")
+    public void 
testSetOfAtomicOperationsWithinTransactionsCheckFalseSystemPropertyFalse() 
throws Exception {
         checkOperations(false);
     }
 
     /**
-     * @param isAtomicCacheAllowedInTx If true - atomic operation allowed.
-     * Otherwise - it should throw exception.
+     * Tests that atomic cache operations within transactions are forbidden if 
system property
+     * {@link IgniteSystemProperties#IGNITE_ALLOW_ATOMIC_OPS_IN_TX 
IGNITE_ALLOW_ATOMIC_OPS_IN_TX}
+     * is changed to {@code true} and the user does not explicitly allow 
atomic operations in transactions via
+     * {@link IgniteCache#withAllowAtomicOpsInTx() withAllowAtomicOpsInTx()} 
before transactions start.
+     * Since 2.15.0 the default behaviour is set to {@code false}.
+     * @throws Exception If failed.
+     */
+    @Test
+    @WithSystemProperty(key = IGNITE_ALLOW_ATOMIC_OPS_IN_TX, value = "true")
+    public void 
testSetOfAtomicOperationsWithinTransactionsCheckFalseSystemPropertyTrue() 
throws Exception {
+        checkOperations(false);
+    }
+
+    /**
+     * @throws IgniteException If failed.
      */
-    private void checkOperations(boolean isAtomicCacheAllowedInTx) {
+    private void checkOperations(boolean withAllowAtomicOpsInTx) {
         HashMap<Integer, Integer> map = new HashMap<>();
 
         map.put(1, 1);
         map.put(2, 1);
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.put(1, 1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.put(1, 1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.putAsync(1, 
1).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.putAsync(1, 
1).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.putAll(map));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.putAll(map));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.putAllAsync(map).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.putAllAsync(map).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.putIfAbsent(1, 
1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.putIfAbsent(1, 
1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.putIfAbsentAsync(1, 1).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.putIfAbsentAsync(1, 1).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.get(1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.get(1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAll(map.keySet()));
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAll(map.keySet()));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAllAsync(map.keySet()).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAllAsync(map.keySet()).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.getAndPut(1, 
2));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.getAndPut(1, 2));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndPutAsync(1, 2).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAndPutAsync(1, 2).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndPutIfAbsent(1, 2));
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAndPutIfAbsent(1, 2));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndPutIfAbsentAsync(1, 2).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAndPutIfAbsentAsync(1, 2).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndRemove(1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.getAndRemove(1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndRemoveAsync(1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAndRemoveAsync(1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndReplace(1, 2));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.getAndReplace(1, 
2));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.getAndReplaceAsync(1, 2).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.getAndReplaceAsync(1, 2).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.remove(1, 1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.remove(1, 1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.removeAsync(1, 
1).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.removeAsync(1, 
1).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.removeAll(map.keySet()));
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.removeAll(map.keySet()));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.removeAllAsync(map.keySet()).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.removeAllAsync(map.keySet()).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.containsKey(1));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.containsKey(1));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.containsKeyAsync(1).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.containsKeyAsync(1).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.containsKeys(map.keySet()));
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.containsKeys(map.keySet()));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.containsKeysAsync(map.keySet()).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.containsKeysAsync(map.keySet()).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.invoke(1, new 
SetEntryProcessor()));
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.invoke(1, new 
SetEntryProcessor()));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> cache.invokeAsync(1, 
new SetEntryProcessor()).get());
+        checkOperation(withAllowAtomicOpsInTx, cache -> cache.invokeAsync(1, 
new SetEntryProcessor()).get());
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.invokeAll(map.keySet(), new SetEntryProcessor()));
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.invokeAll(map.keySet(), new SetEntryProcessor()));
 
-        checkOperation(isAtomicCacheAllowedInTx, cache -> 
cache.invokeAllAsync(map.keySet(),
+        checkOperation(withAllowAtomicOpsInTx, cache -> 
cache.invokeAllAsync(map.keySet(),
             new SetEntryProcessor()).get());
-
-        checkLock(isAtomicCacheAllowedInTx);

Review Comment:
   Why did you remove this line?



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to