IGNITE-24076 Review fixes

apache · Dec 25, 2024 · a6a73cb · a6a73cb
1 parent 01afca4
commit a6a73cb
Show file tree

Hide file tree

Showing 9 changed files with 32 additions and 40 deletions.
diff --git a/.../core/src/main/java/org/apache/ignite/internal/cluster/DistributedConfigurationUtils.java b/.../core/src/main/java/org/apache/ignite/internal/cluster/DistributedConfigurationUtils.java
@@ -33,7 +33,6 @@
 import org.apache.ignite.internal.processors.subscription.GridInternalSubscriptionProcessor;
 import org.apache.ignite.internal.util.future.GridFinishedFuture;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
 import static java.lang.String.format;
 
@@ -141,12 +140,4 @@ public static List<DistributedBooleanProperty> newConnectionEnabledProperty(
 
         return props;
     }
-
-    /**
-     * @param prop Property to get value from.
-     * @return Property value. Default {@code true}.
-     */
-    public static boolean asBoolean(@Nullable DistributedBooleanProperty prop) {
-        return prop != null ? prop.getOrDefault(true) : Boolean.TRUE;
-    }
 }
diff --git a/.../ignite/internal/processors/cache/persistence/wal/reader/StandaloneGridKernalContext.java b/.../ignite/internal/processors/cache/persistence/wal/reader/StandaloneGridKernalContext.java
@@ -677,7 +677,7 @@ private void setField(IgniteEx kernal, String name, Object val) throws NoSuchFie
 
     /** {@inheritDoc} */
     @Override public GridInternalSubscriptionProcessor internalSubscriptionProcessor() {
-        return new GridInternalSubscriptionProcessor(this);
+        return null;
     }
 
     /** {@inheritDoc} */

diff --git a/...main/java/org/apache/ignite/internal/processors/odbc/ClientListenerConnectionContext.java b/...main/java/org/apache/ignite/internal/processors/odbc/ClientListenerConnectionContext.java
@@ -94,9 +94,7 @@ void initializeFromHandshake(GridNioSession ses, ClientListenerProtocolVersion v
     /**
      * @return {@code True} if client is management.
      */
-    default boolean isManagementClient() {
-        Map<String, String> attrs = attributes();
-
-        return attrs != null && Boolean.parseBoolean(attrs.get(MANAGEMENT_CLIENT_ATTR));
+    default boolean managementClient() {
+        return Boolean.parseBoolean(attributes().get(MANAGEMENT_CLIENT_ATTR));
     }
 }
diff --git a/...e/src/main/java/org/apache/ignite/internal/processors/odbc/ClientListenerNioListener.java b/...e/src/main/java/org/apache/ignite/internal/processors/odbc/ClientListenerNioListener.java
@@ -115,7 +115,7 @@ public class ClientListenerNioListener extends GridNioServerListenerAdapter<Clie
      * @see ClientListenerNioListener#JDBC_CLIENT
      * @see ClientListenerNioListener#THIN_CLIENT
      */
-    private final Predicate<Byte> newConnectionEnabled;
+    private final Predicate<Byte> newConnEnabled;
 
     /**
      * Constructor.
@@ -124,14 +124,14 @@ public class ClientListenerNioListener extends GridNioServerListenerAdapter<Clie
      * @param busyLock Shutdown busy lock.
      * @param cliConnCfg Client connector configuration.
      * @param metrics Client listener metrics.
-     * @param newConnectionEnabled Predicate to check if connection of specified type enabled.
+     * @param newConnEnabled Predicate to check if connection of specified type enabled.
      */
     public ClientListenerNioListener(
         GridKernalContext ctx,
         GridSpinBusyLock busyLock,
         ClientConnectorConfiguration cliConnCfg,
         ClientListenerMetrics metrics,
-        Predicate<Byte> newConnectionEnabled
+        Predicate<Byte> newConnEnabled
     ) {
         assert cliConnCfg != null;
 
@@ -147,7 +147,7 @@ public ClientListenerNioListener(
             : new ThinClientConfiguration(cliConnCfg.getThinClientConfiguration());
 
         this.metrics = metrics;
-        this.newConnectionEnabled = newConnectionEnabled;
+        this.newConnEnabled = newConnEnabled;
     }
 
     /** {@inheritDoc} */
@@ -559,7 +559,7 @@ private void ensureClientPermissions(byte clientType) throws IgniteCheckedExcept
      * @throws IgniteCheckedException If failed.
      */
     private void ensureConnectionAllowed(ClientListenerConnectionContext connCtx) throws IgniteCheckedException {
-        boolean isControlUtility = connCtx.clientType() == THIN_CLIENT && connCtx.isManagementClient();
+        boolean isControlUtility = connCtx.clientType() == THIN_CLIENT && connCtx.managementClient();
 
         if (nodeInRecoveryMode()) {
             if (!isControlUtility)
@@ -568,7 +568,7 @@ private void ensureConnectionAllowed(ClientListenerConnectionContext connCtx) th
             return;
         }
 
-        // If security enabled then only admin allowed to connect as management
+        // If security enabled then only admin allowed to connect as management.
         if (isControlUtility) {
             if (connCtx.securityContext() != null) {
                 try (OperationSecurityContext ignored = ctx.security().withContext(connCtx.securityContext())) {
@@ -584,7 +584,7 @@ private void ensureConnectionAllowed(ClientListenerConnectionContext connCtx) th
             return;
         }
 
-        if (!newConnectionEnabled.test(connCtx.clientType()))
+        if (!newConnEnabled.test(connCtx.clientType()))
             throw new IgniteAccessControlException(CONN_DISABLED_BY_ADMIN_ERR_MSG);
     }
 }
diff --git a/...ore/src/main/java/org/apache/ignite/internal/processors/odbc/ClientListenerProcessor.java b/...ore/src/main/java/org/apache/ignite/internal/processors/odbc/ClientListenerProcessor.java
@@ -69,7 +69,6 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import static org.apache.ignite.internal.cluster.DistributedConfigurationUtils.asBoolean;
 import static org.apache.ignite.internal.cluster.DistributedConfigurationUtils.newConnectionEnabledProperty;
 import static org.apache.ignite.internal.processors.metric.GridMetricManager.CLIENT_CONNECTOR_METRICS;
 import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.metricName;
@@ -185,14 +184,14 @@ public ClientListenerProcessor(GridKernalContext ctx) {
                         ? this::onOutboundMessageOffered
                         : null;
 
-                Predicate<Byte> newConnectionEnabled = connectionEnabledPredicate();
+                Predicate<Byte> newConnEnabled = connectionEnabledPredicate();
 
                 for (int port = cliConnCfg.getPort(); port <= portTo && port <= 65535; port++) {
                     try {
                         srv = GridNioServer.<ClientMessage>builder()
                             .address(hostAddr)
                             .port(port)
-                            .listener(new ClientListenerNioListener(ctx, busyLock, cliConnCfg, metrics, newConnectionEnabled))
+                            .listener(new ClientListenerNioListener(ctx, busyLock, cliConnCfg, metrics, newConnEnabled))
                             .logger(log)
                             .selectorCount(selectorCnt)
                             .igniteInstanceName(ctx.igniteInstanceName())
@@ -265,7 +264,7 @@ public ClientListenerProcessor(GridKernalContext ctx) {
      * @see ClientListenerNioListener#THIN_CLIENT
      */
     private Predicate<Byte> connectionEnabledPredicate() {
-        Map<Byte, DistributedBooleanProperty> allowConnMap = new HashMap<>();
+        Map<Byte, DistributedBooleanProperty> сonnEnabledMap = new HashMap<>();
 
         List<DistributedBooleanProperty> props = newConnectionEnabledProperty(
             ctx.internalSubscriptionProcessor(),
@@ -275,15 +274,15 @@ private Predicate<Byte> connectionEnabledPredicate() {
             "Thin"
         );
 
-        allowConnMap.put(ODBC_CLIENT, props.get(0));
-        allowConnMap.put(JDBC_CLIENT, props.get(1));
-        allowConnMap.put(THIN_CLIENT, props.get(2));
+        сonnEnabledMap.put(ODBC_CLIENT, props.get(0));
+        сonnEnabledMap.put(JDBC_CLIENT, props.get(1));
+        сonnEnabledMap.put(THIN_CLIENT, props.get(2));
 
         return type -> {
             assert type != null : "Connection type is null";
-            assert allowConnMap.containsKey(type) : "Unknown connection type: " + type;
+            assert сonnEnabledMap.containsKey(type) : "Unknown connection type: " + type;
 
-            return asBoolean(allowConnMap.get(type));
+            return сonnEnabledMap.get(type).getOrDefault(true);
         };
     }
 

diff --git a/modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java b/modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ServerImpl.java
@@ -179,7 +179,6 @@
 import static org.apache.ignite.internal.IgniteNodeAttributes.ATTR_MARSHALLER_USE_BINARY_STRING_SER_VER_2;
 import static org.apache.ignite.internal.IgniteNodeAttributes.ATTR_MARSHALLER_USE_DFLT_SUID;
 import static org.apache.ignite.internal.cluster.DistributedConfigurationUtils.CONN_DISABLED_BY_ADMIN_ERR_MSG;
-import static org.apache.ignite.internal.cluster.DistributedConfigurationUtils.asBoolean;
 import static org.apache.ignite.internal.cluster.DistributedConfigurationUtils.newConnectionEnabledProperty;
 import static org.apache.ignite.internal.processors.security.SecurityUtils.authenticateLocalNode;
 import static org.apache.ignite.internal.processors.security.SecurityUtils.withSecurityContext;
@@ -302,10 +301,10 @@ class ServerImpl extends TcpDiscoveryImpl {
         new ConcurrentHashMap<>();
 
     /** Client node connection allowed property. */
-    private DistributedBooleanProperty clientConnectionEnabled;
+    private final DistributedBooleanProperty cliConnEnabled;
 
     /** Server node connection allowed property. */
-    private DistributedBooleanProperty serverConnectionEnabled;
+    private final DistributedBooleanProperty srvConnEnabled;
 
     /**
      * Maximum size of history of IDs of server nodes ever tried to join current topology (ever sent join request).
@@ -337,8 +336,8 @@ class ServerImpl extends TcpDiscoveryImpl {
             "ServerNode"
         );
 
-        clientConnectionEnabled = props.get(0);
-        serverConnectionEnabled = props.get(1);
+        cliConnEnabled = props.get(0);
+        srvConnEnabled = props.get(1);
     }
 
     /** {@inheritDoc} */
@@ -6448,7 +6447,7 @@ private void checkConnection() {
      * @return {@code null} if connection allowed, error otherwise.
      */
     private IgniteNodeValidationResult ensureJoinEnabled(TcpDiscoveryNode node) {
-        return asBoolean(node.isClient() ? clientConnectionEnabled : serverConnectionEnabled)
+        return (node.isClient() ? cliConnEnabled : srvConnEnabled).getOrDefault(true)
             ? null
             : new IgniteNodeValidationResult(node.id(), CONN_DISABLED_BY_ADMIN_ERR_MSG);
     }

diff --git a/...che/ignite/internal/processors/security/client/ThinClientPermissionCheckSecurityTest.java b/...che/ignite/internal/processors/security/client/ThinClientPermissionCheckSecurityTest.java
@@ -51,7 +51,7 @@ public class ThinClientPermissionCheckSecurityTest extends ThinClientPermissionC
     }
 
     /** {@inheritDoc} */
-    @Override public void testConnectAsManagementClient() throws Exception {
+    @Override public void testConnectAsManagementClient() {
         // No-op.
     }
 }
diff --git a/.../org/apache/ignite/internal/processors/security/client/ThinClientPermissionCheckTest.java b/.../org/apache/ignite/internal/processors/security/client/ThinClientPermissionCheckTest.java
@@ -390,7 +390,7 @@ public void testCacheEntryEviction() throws Exception {
     /** */
     @Test
     public void testConnectAsManagementClient() throws Exception {
-        Runnable clientCanConnect = () -> {
+        Runnable cliCanConnect = () -> {
             try (IgniteClient cli = startClient(CLIENT)) {
                 assertNotNull("Cach query from CLIENT", cli.cacheNames());
             }
@@ -418,7 +418,7 @@ public void testConnectAsManagementClient() throws Exception {
         };
 
         Runnable checkDflt = () -> {
-            clientCanConnect.run();
+            cliCanConnect.run();
             adminCanConnect.run();
 
             withUserAttrsCheck.run();

diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/IgniteMock.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/IgniteMock.java
@@ -72,6 +72,7 @@
 import org.apache.ignite.internal.processors.cache.IgniteInternalCache;
 import org.apache.ignite.internal.processors.cache.persistence.wal.reader.StandaloneGridKernalContext;
 import org.apache.ignite.internal.processors.cacheobject.NoOpBinary;
+import org.apache.ignite.internal.processors.subscription.GridInternalSubscriptionProcessor;
 import org.apache.ignite.internal.processors.tracing.configuration.NoopTracingConfigurationManager;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.lang.IgniteBiTuple;
@@ -142,7 +143,11 @@ public IgniteMock(
         this.staticCfg = staticCfg;
 
         try {
-            kernalCtx = new StandaloneGridKernalContext(new GridTestLog4jLogger(), null, null);
+            kernalCtx = new StandaloneGridKernalContext(new GridTestLog4jLogger(), null, null) {
+                @Override public GridInternalSubscriptionProcessor internalSubscriptionProcessor() {
+                    return new GridInternalSubscriptionProcessor(this);
+                }
+            };
         }
         catch (IgniteCheckedException e) {
             throw new IgniteException(e);