mklaca commented on a change in pull request #76:
URL: https://github.com/apache/qpid-broker-j/pull/76#discussion_r649950920



##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/model/NamedAddressSpace.java
##########
@@ -48,8 +49,13 @@
 
     boolean registerConnection(AMQPConnection<?> connection,
                                final ConnectionEstablishmentPolicy 
connectionEstablishmentPolicy);
+
     void deregisterConnection(AMQPConnection<?> connection);
 
+    default ConnectionSlot requestConnectionSlot(AMQPConnection<?> connection)

Review comment:
       ConnectionSlot is a crucial part of the solution, see my last comment.

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/security/limit/ConnectionSlot.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.limit;
+
+import java.util.Optional;
+
+@FunctionalInterface
+public interface ConnectionSlot extends Runnable

Review comment:
       ConnectionSlot is a crucial part of the solution, it can be used as 
delete task.

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/security/limit/ConnectionLimiter.java
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.limit;
+
+import java.util.Optional;
+
+import org.apache.qpid.server.security.limit.ConnectionSlot.FreeSlot;
+import org.apache.qpid.server.transport.AMQPConnection;
+
+public interface ConnectionLimiter
+{
+    ConnectionSlot register(AMQPConnection<?> connection);
+
+    ConnectionLimiter append(ConnectionLimiter limiter);
+
+    static CachedLimiter noLimits()
+    {
+        return NoLimits.INSTANCE;
+    }
+
+    static CachedLimiter blockEveryone()
+    {
+        return BlockEveryone.INSTANCE;
+    }
+
+    static CachedLimiter cacheConnectionRegistration(ConnectionLimiter 
limiter) {
+        if (limiter instanceof CachedLimiter) {
+            return (CachedLimiter) limiter;
+        }
+        return new CachedConnectionLimiterImpl(limiter);
+    }
+
+    interface CachedLimiter extends ConnectionLimiter
+    {
+        ConnectionLimiter underlyingLimiter();
+
+        @Override
+        default ConnectionLimiter append(ConnectionLimiter limiter) {
+            return underlyingLimiter().append(limiter);
+        }
+    }
+
+    final class NoLimits implements CachedLimiter

Review comment:
       Because NoLimits  is a CachedLimiter.

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/security/limit/ConnectionLimiter.java
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.qpid.server.security.limit;
+
+import java.util.Optional;
+
+import org.apache.qpid.server.security.limit.ConnectionSlot.FreeSlot;
+import org.apache.qpid.server.transport.AMQPConnection;
+
+public interface ConnectionLimiter
+{
+    ConnectionSlot register(AMQPConnection<?> connection);
+
+    ConnectionLimiter append(ConnectionLimiter limiter);
+
+    static CachedLimiter noLimits()
+    {
+        return NoLimits.INSTANCE;
+    }
+
+    static CachedLimiter blockEveryone()
+    {
+        return BlockEveryone.INSTANCE;
+    }
+
+    static CachedLimiter cacheConnectionRegistration(ConnectionLimiter 
limiter) {
+        if (limiter instanceof CachedLimiter) {
+            return (CachedLimiter) limiter;
+        }
+        return new CachedConnectionLimiterImpl(limiter);
+    }
+
+    interface CachedLimiter extends ConnectionLimiter
+    {
+        ConnectionLimiter underlyingLimiter();
+
+        @Override
+        default ConnectionLimiter append(ConnectionLimiter limiter) {
+            return underlyingLimiter().append(limiter);
+        }
+    }
+
+    final class NoLimits implements CachedLimiter
+    {
+        static CachedLimiter INSTANCE = new NoLimits();
+
+        private NoLimits()
+        {
+            super();
+        }
+
+        @Override
+        public ConnectionSlot register(AMQPConnection<?> connection)
+        {
+            return FreeSlot.INSTANCE;
+        }
+
+        @Override
+        public ConnectionLimiter append(ConnectionLimiter limiter)
+        {
+            return Optional.ofNullable(limiter).orElse(this);
+        }
+
+        @Override
+        public ConnectionLimiter underlyingLimiter()
+        {
+            return this;
+        }
+    }
+
+    final class BlockEveryone implements CachedLimiter

Review comment:
       Because BlockEveryone is a CachedLimiter.

##########
File path: 
broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
##########
@@ -2691,6 +2704,12 @@ public String getArguments()
         });
     }
 
+    @Override
+    public ConnectionSlot requestConnectionSlot(AMQPConnection<?> connection)
+    {
+        return _connectionLimiter.register(connection);

Review comment:
       See my last comment.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to