Repository: mina-sshd Updated Branches: refs/heads/master 340803bb3 -> 9c38dc564
[SSHD-478] Define FactoryManager properties to be <String,Object> Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/9c38dc56 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/9c38dc56 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/9c38dc56 Branch: refs/heads/master Commit: 9c38dc56488eac0d45868595a30bb8604503c782 Parents: 340803b Author: Lyor Goldstein <[email protected]> Authored: Sun May 31 13:51:04 2015 +0300 Committer: Lyor Goldstein <[email protected]> Committed: Sun May 31 13:51:04 2015 +0300 ---------------------------------------------------------------------- .../sshd/common/AbstractFactoryManager.java | 8 +- .../org/apache/sshd/common/FactoryManager.java | 15 +- .../apache/sshd/common/FactoryManagerUtils.java | 57 +++---- .../sshd/common/session/AbstractSession.java | 23 ++- .../apache/sshd/server/sftp/SftpSubsystem.java | 13 +- .../src/test/java/org/apache/sshd/LoadTest.java | 2 +- .../java/org/apache/sshd/client/ScpTest.java | 1 - .../sshd/common/FactoryManagerUtilsTest.java | 157 +++++++++++++++++++ .../io/DefaultIoServiceFactoryFactoryTest.java | 2 +- 9 files changed, 211 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/main/java/org/apache/sshd/common/AbstractFactoryManager.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/AbstractFactoryManager.java b/sshd-core/src/main/java/org/apache/sshd/common/AbstractFactoryManager.java index a4dc257..8c0e32f 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/AbstractFactoryManager.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/AbstractFactoryManager.java @@ -49,7 +49,7 @@ import org.apache.sshd.common.util.threads.ThreadUtils; */ public abstract class AbstractFactoryManager extends CloseableUtils.AbstractInnerCloseable implements FactoryManager { - protected Map<String,String> properties = new HashMap<String,String>(); + protected Map<String,Object> properties = new HashMap<String,Object>(); protected IoServiceFactoryFactory ioServiceFactoryFactory; protected IoServiceFactory ioServiceFactory; protected List<NamedFactory<KeyExchange>> keyExchangeFactories; @@ -158,12 +158,12 @@ public abstract class AbstractFactoryManager extends CloseableUtils.AbstractInne } @Override - public Map<String, String> getProperties() { + public Map<String, Object> getProperties() { return properties; } - public void setProperties(Map<String, String> properties) { - this.properties = properties; + public void setProperties(Map<String, Object> properties) { + this.properties = ValidateUtils.checkNotNull(properties, "Null properties not allowed", GenericUtils.EMPTY_OBJECT_ARRAY); } @Override http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java b/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java index f8b5508..172eac4 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/FactoryManager.java @@ -144,16 +144,23 @@ public interface FactoryManager { public static final String NIO2_READ_BUFFER_SIZE = "nio2-read-buf-size"; /** - * A map of properties that can be used to configure the SSH server + * <P>A map of properties that can be used to configure the SSH server * or client. This map will never be changed by either the server or * client and is not supposed to be changed at runtime (changes are not * bound to have any effect on a running client or server), though it may * affect the creation of sessions later as these values are usually not - * cached. - * + * cached.</P></BR> + * <B>Note:</B> the <U>type</U> of the mapped property should match the + * expected configuration value type - {@code Long, Integer, Boolean, + * String, etc...). If it doesn't, the {@code toString()} result of the + * mapped value is used to convert it to the required type. E.g., if + * the mapped value is the <U>string</U> "1234" and the expected + * value is a {@code long} then it will be parsed into one. Also, if + * the mapped value is an {@code Integer} but a {@code long} is expected, + * then it will be converted into one. * @return a valid <code>Map</code> containing configuration values, never <code>null</code> */ - Map<String,String> getProperties(); + Map<String,Object> getProperties(); /** * An upper case string identifying the version of the http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/main/java/org/apache/sshd/common/FactoryManagerUtils.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/FactoryManagerUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/FactoryManagerUtils.java index bab4bc0..a8ee0cc 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/FactoryManagerUtils.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/FactoryManagerUtils.java @@ -20,7 +20,6 @@ package org.apache.sshd.common; import java.util.Map; -import java.util.Objects; import org.apache.sshd.common.util.GenericUtils; @@ -109,16 +108,16 @@ public class FactoryManagerUtils { } } - public static final String updateProperty(Session session, String name, long value) { + public static final Object updateProperty(Session session, String name, long value) { return updateProperty(session, name, Long.toString(value)); } - public static final String updateProperty(FactoryManager manager, String name, long value) { - return updateProperty(manager, name, Long.toString(value)); + public static final Object updateProperty(FactoryManager manager, String name, long value) { + return updateProperty(manager.getProperties(), name, value); } - public static final String updateProperty(Map<String,String> props, String name, long value) { - return updateProperty(props, name, Long.toString(value)); + public static final Object updateProperty(Map<String,Object> props, String name, long value) { + return updateProperty(props, name, Long.valueOf(value)); } public static final int getIntProperty(Session session, String name, int defaultValue) { @@ -159,16 +158,16 @@ public class FactoryManagerUtils { } } - public static final String updateProperty(Session session, String name, int value) { - return updateProperty(session, name, Integer.toString(value)); + public static final Object updateProperty(Session session, String name, int value) { + return updateProperty(session.getFactoryManager(), name, value); } - public static final String updateProperty(FactoryManager manager, String name, int value) { - return updateProperty(manager, name, Integer.toString(value)); + public static final Object updateProperty(FactoryManager manager, String name, int value) { + return updateProperty(manager.getProperties(), name, value); } - public static final String updateProperty(Map<String, String> props, String name, int value) { - return updateProperty(props, name, Integer.toString(value)); + public static final Object updateProperty(Map<String,Object> props, String name, int value) { + return updateProperty(props, name, Integer.valueOf(value)); } public static final boolean getBooleanProperty(Session session, String name, boolean defaultValue) { @@ -207,16 +206,16 @@ public class FactoryManagerUtils { } } - public static final String updateProperty(Session session, String name, boolean value) { - return updateProperty(session, name, Boolean.toString(value)); + public static final Object updateProperty(Session session, String name, boolean value) { + return updateProperty(session.getFactoryManager(), name, value); } - public static final String updateProperty(FactoryManager manager, String name, boolean value) { - return updateProperty(manager, name, Boolean.toString(value)); + public static final Object updateProperty(FactoryManager manager, String name, boolean value) { + return updateProperty(manager.getProperties(), name, value); } - public static final String updateProperty(Map<String, String> props, String name, boolean value) { - return updateProperty(props, name, Boolean.toString(value)); + public static final Object updateProperty(Map<String,Object> props, String name, boolean value) { + return updateProperty(props, name, Boolean.valueOf(value)); } public static final String getString(Session session, String name) { @@ -249,23 +248,11 @@ public class FactoryManagerUtils { } } - public static String updateProperty(Session session, String name, Object value) { - return updateProperty(session.getFactoryManager(), name, value); - } - - public static String updateProperty(FactoryManager manager, String name, Object value) { - return updateProperty(manager.getProperties(), name, value); - } - - public static String updateProperty(Map<String, String> props, String name, Object value) { - return updateProperty(props, name, Objects.toString(value)); - } - - public static String updateProperty(Session session, String name, String value) { + public static Object updateProperty(Session session, String name, Object value) { return updateProperty(session.getFactoryManager(), name, value); } - public static String updateProperty(FactoryManager manager, String name, String value) { + public static Object updateProperty(FactoryManager manager, String name, Object value) { return updateProperty(manager.getProperties(), name, value); } @@ -273,11 +260,11 @@ public class FactoryManagerUtils { * @param props The {@link Map} of properties to update * @param name The property name * @param value The property value - if {@code null}/empty then the - * specified property is <U>removed</U> from the properties map + * specified property is <U>removed</U> from the properties map * @return The removed or previous value (if any) */ - public static String updateProperty(Map<String, String> props, String name, String value) { - if (GenericUtils.isEmpty(value)) { + public static Object updateProperty(Map<String,Object> props, String name, Object value) { + if ((value == null) || ((value instanceof CharSequence) && GenericUtils.isEmpty((CharSequence) value))) { return props.remove(name); } else { return props.put(name, value); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java index 98c3f6e..62af8da 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/session/AbstractSession.java @@ -45,6 +45,7 @@ import org.apache.sshd.common.Cipher; import org.apache.sshd.common.Closeable; import org.apache.sshd.common.Digest; import org.apache.sshd.common.FactoryManager; +import org.apache.sshd.common.FactoryManagerUtils; import org.apache.sshd.common.KeyExchange; import org.apache.sshd.common.Mac; import org.apache.sshd.common.NamedFactory; @@ -1235,26 +1236,24 @@ public abstract class AbstractSession extends CloseableUtils.AbstractInnerClosea @Override public int getIntProperty(String name, int defaultValue) { try { - String v = factoryManager.getProperties().get(name); - if (v != null) { - return Integer.parseInt(v); - } + return FactoryManagerUtils.getIntProperty(factoryManager, name, defaultValue); } catch (Exception e) { - // Ignore + if (log.isDebugEnabled()) { + log.debug("getIntProperty(" + name + ") failed (" + e.getClass().getSimpleName() + ") to retrieve: " + e.getMessage()); + } + return defaultValue; } - return defaultValue; } public long getLongProperty(String name, long defaultValue) { try { - String v = factoryManager.getProperties().get(name); - if (v != null) { - return Long.parseLong(v); - } + return FactoryManagerUtils.getLongProperty(factoryManager, name, defaultValue); } catch (Exception e) { - // Ignore + if (log.isDebugEnabled()) { + log.debug("getLongProperty(" + name + ") failed (" + e.getClass().getSimpleName() + ") to retrieve: " + e.getMessage()); + } + return defaultValue; } - return defaultValue; } /** http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/main/java/org/apache/sshd/server/sftp/SftpSubsystem.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/server/sftp/SftpSubsystem.java b/sshd-core/src/main/java/org/apache/sshd/server/sftp/SftpSubsystem.java index 0fdb213..b57705d 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/sftp/SftpSubsystem.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/sftp/SftpSubsystem.java @@ -1154,15 +1154,10 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna } protected void doOpen(Buffer buffer, int id) throws IOException { - if (session.getFactoryManager().getProperties() != null) { - String maxHandlesString = session.getFactoryManager().getProperties().get(MAX_OPEN_HANDLES_PER_SESSION); - if (maxHandlesString != null) { - int maxHandleCount = Integer.parseInt(maxHandlesString); - if (handles.size() > maxHandleCount) { - sendStatus(id, SSH_FX_FAILURE, "Too many open handles"); - return; - } - } + int maxHandleCount = FactoryManagerUtils.getIntProperty(session, MAX_OPEN_HANDLES_PER_SESSION, Integer.MAX_VALUE); + if (handles.size() > maxHandleCount) { + sendStatus(id, SSH_FX_FAILURE, "Too many open handles"); + return; } String path = buffer.getString(); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/test/java/org/apache/sshd/LoadTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/LoadTest.java b/sshd-core/src/test/java/org/apache/sshd/LoadTest.java index a756ef6..df5686c 100644 --- a/sshd-core/src/test/java/org/apache/sshd/LoadTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/LoadTest.java @@ -112,7 +112,7 @@ public class LoadTest extends BaseTestSupport { protected void runClient(String msg) throws Exception { try(SshClient client = SshClient.setUpDefaultClient()) { - Map<String,String> props=client.getProperties(); + Map<String,Object> props=client.getProperties(); FactoryManagerUtils.updateProperty(props, FactoryManager.MAX_PACKET_SIZE, 1024 * 16); FactoryManagerUtils.updateProperty(props, FactoryManager.WINDOW_SIZE, 1024 * 8); client.setKeyExchangeFactories(Arrays.asList( http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/test/java/org/apache/sshd/client/ScpTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ScpTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ScpTest.java index 0a5406b..a170ad5 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/ScpTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/ScpTest.java @@ -18,7 +18,6 @@ */ package org.apache.sshd.client; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/test/java/org/apache/sshd/common/FactoryManagerUtilsTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/common/FactoryManagerUtilsTest.java b/sshd-core/src/test/java/org/apache/sshd/common/FactoryManagerUtilsTest.java new file mode 100644 index 0000000..fa5aae0 --- /dev/null +++ b/sshd-core/src/test/java/org/apache/sshd/common/FactoryManagerUtilsTest.java @@ -0,0 +1,157 @@ +/* + * 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.sshd.common; + +import java.util.Map; +import java.util.TreeMap; + +import org.apache.sshd.util.BaseTestSupport; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * @author <a href="mailto:[email protected]";>Apache MINA SSHD Project</a> + */ +public class FactoryManagerUtilsTest extends BaseTestSupport { + public FactoryManagerUtilsTest() { + super(); + } + + @Test + public void testLongProperty() { + final long expected=System.currentTimeMillis(); + final String name=getCurrentTestName(); + + Session session = createMockSession(); + assertEquals("Mismatched empty props value", expected, FactoryManagerUtils.getLongProperty(session, name, expected)); + + FactoryManagerUtils.updateProperty(session, name, expected); + testLongProperty(session, name, expected); + + FactoryManagerUtils.updateProperty(session, name, Long.toString(expected)); + testLongProperty(session, name, expected); + } + + private void testLongProperty(Session session, String name, long expected) { + FactoryManager manager = session.getFactoryManager(); + Map<String,?> props = manager.getProperties(); + Object value = props.get(name); + Class<?> type = value.getClass(); + String storage = type.getSimpleName(); + + { + Long actual = FactoryManagerUtils.getLong(session, name); + assertNotNull("No actual Long value found for storage as " + storage, actual); + assertEquals("Mismatched values on Long retrieval for storage as " + storage, expected, actual.longValue()); + } + + { + String actual = FactoryManagerUtils.getString(session, name); + assertNotNull("No actual String value found for storage as " + storage, actual); + assertEquals("Mismatched values on String retrieval for storage as " + storage, Long.toString(expected), actual.toString()); + } + } + + @Test + public void testIntegerProperty() { + final int expected=3777347; + final String name=getCurrentTestName(); + + Session session = createMockSession(); + assertEquals("Mismatched empty props value", expected, FactoryManagerUtils.getIntProperty(session, name, expected)); + + FactoryManagerUtils.updateProperty(session, name, expected); + testIntegerProperty(session, name, expected); + + FactoryManagerUtils.updateProperty(session, name, Integer.toString(expected)); + testIntegerProperty(session, name, expected); + + // store as Long but retrieve as Integer + FactoryManagerUtils.updateProperty(session, name, Long.valueOf(expected)); + testIntegerProperty(session, name, expected); + } + + private void testIntegerProperty(Session session, String name, int expected) { + FactoryManager manager = session.getFactoryManager(); + Map<String,?> props = manager.getProperties(); + Object value = props.get(name); + Class<?> type = value.getClass(); + String storage = type.getSimpleName(); + + { + Integer actual = FactoryManagerUtils.getInteger(session, name); + assertNotNull("No actual Long value found for storage as " + storage, actual); + assertEquals("Mismatched values on Long retrieval for storage as " + storage, expected, actual.intValue()); + } + + { + String actual = FactoryManagerUtils.getString(session, name); + assertNotNull("No actual String value found for storage as " + storage, actual); + assertEquals("Mismatched values on String retrieval for storage as " + storage, Long.toString(expected), actual.toString()); + } + } + + @Test + public void testBooleanProperty() { + for (final boolean expected : new boolean[] { false, true }) { + final String name=getCurrentTestName(); + + Session session = createMockSession(); + assertEquals("Mismatched empty props value", expected, FactoryManagerUtils.getBooleanProperty(session, name, expected)); + + FactoryManagerUtils.updateProperty(session, name, expected); + testBooleanProperty(session, name, expected); + + FactoryManagerUtils.updateProperty(session, name, Boolean.toString(expected)); + testBooleanProperty(session, name, expected); + } + } + + private void testBooleanProperty(Session session, String name, boolean expected) { + FactoryManager manager = session.getFactoryManager(); + Map<String,?> props = manager.getProperties(); + Object value = props.get(name); + Class<?> type = value.getClass(); + String storage = type.getSimpleName(); + + { + Boolean actual = FactoryManagerUtils.getBoolean(session, name); + assertNotNull("No actual Long value found for storage as " + storage, actual); + assertEquals("Mismatched values on Long retrieval for storage as " + storage, expected, actual.booleanValue()); + } + + { + String actual = FactoryManagerUtils.getString(session, name); + assertNotNull("No actual String value found for storage as " + storage, actual); + assertEquals("Mismatched values on String retrieval for storage as " + storage, Boolean.toString(expected), actual.toString()); + } + } + + private Session createMockSession() { + Map<String,Object> props = new TreeMap<String,Object>(String.CASE_INSENSITIVE_ORDER); + FactoryManager manager = Mockito.mock(FactoryManager.class); + Mockito.when(manager.getProperties()).thenReturn(props); + + Session session = Mockito.mock(Session.class); + Mockito.when(session.getUsername()).thenReturn(getCurrentTestName()); + Mockito.when(session.getFactoryManager()).thenReturn(manager); + return session; + } +} http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9c38dc56/sshd-core/src/test/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactoryTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactoryTest.java b/sshd-core/src/test/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactoryTest.java index dea271d..eedea5f 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactoryTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactoryTest.java @@ -57,7 +57,7 @@ public class DefaultIoServiceFactoryFactoryTest extends BaseTestSupport { Mockito.when(service.isTerminated()).thenReturn(Boolean.TRUE); FactoryManager manager = Mockito.mock(FactoryManager.class); - Mockito.when(manager.getProperties()).thenReturn(Collections.<String,String>emptyMap()); + Mockito.when(manager.getProperties()).thenReturn(Collections.<String,Object>emptyMap()); String propName = IoServiceFactoryFactory.class.getName(); for (BuiltinIoServiceFactoryFactories f : BuiltinIoServiceFactoryFactories.VALUES) {
