From 0234878c4ba0999a464a1acee2983d6cba9de940 Mon Sep 17 00:00:00 2001 From: Nikolay Fedorovskikh Date: Tue, 15 Aug 2017 23:23:35 +0500 Subject: [PATCH] Fix the commands cache and hashCode() in SmtpCommand Motivation: - A `hashCode` of the SmtpCommand is recalculated on each call of `hashCode()`. Cached hash code value can be just replaced with call of `name.hashCode()`. - The commands cache don't work for strings: `SmtpCommand.valueOf("HELO")` returns a new instance. - Field `contentExpected` is redundant and can be replaced with `equals(DATA)`. Modifications: - Use the `name.hashCode()` as hash code result. - Fix a command cache: use strings as map keys. - Replace field `contentExpected` to using `this.equals(DATA)`. - Add unit tests. Result: More correct and clean code. --- .../netty/handler/codec/smtp/SmtpCommand.java | 76 ++++++++----------- .../handler/codec/smtp/SmtpCommandTest.java | 45 +++++++++++ 2 files changed, 76 insertions(+), 45 deletions(-) create mode 100644 codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpCommandTest.java diff --git a/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpCommand.java b/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpCommand.java index acf6c74900..63ba3f5847 100644 --- a/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpCommand.java +++ b/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpCommand.java @@ -29,53 +29,46 @@ import java.util.Map; */ @UnstableApi public final class SmtpCommand { - public static final SmtpCommand EHLO = new SmtpCommand(AsciiString.cached("EHLO"), false); - public static final SmtpCommand HELO = new SmtpCommand(AsciiString.cached("HELO"), false); - public static final SmtpCommand MAIL = new SmtpCommand(AsciiString.cached("MAIL"), false); - public static final SmtpCommand RCPT = new SmtpCommand(AsciiString.cached("RCPT"), false); - public static final SmtpCommand DATA = new SmtpCommand(AsciiString.cached("DATA"), true); - public static final SmtpCommand NOOP = new SmtpCommand(AsciiString.cached("NOOP"), false); - public static final SmtpCommand RSET = new SmtpCommand(AsciiString.cached("RSET"), false); - public static final SmtpCommand EXPN = new SmtpCommand(AsciiString.cached("EXPN"), false); - public static final SmtpCommand VRFY = new SmtpCommand(AsciiString.cached("VRFY"), false); - public static final SmtpCommand HELP = new SmtpCommand(AsciiString.cached("HELP"), false); - public static final SmtpCommand QUIT = new SmtpCommand(AsciiString.cached("QUIT"), false); + public static final SmtpCommand EHLO = new SmtpCommand(AsciiString.cached("EHLO")); + public static final SmtpCommand HELO = new SmtpCommand(AsciiString.cached("HELO")); + public static final SmtpCommand MAIL = new SmtpCommand(AsciiString.cached("MAIL")); + public static final SmtpCommand RCPT = new SmtpCommand(AsciiString.cached("RCPT")); + public static final SmtpCommand DATA = new SmtpCommand(AsciiString.cached("DATA")); + public static final SmtpCommand NOOP = new SmtpCommand(AsciiString.cached("NOOP")); + public static final SmtpCommand RSET = new SmtpCommand(AsciiString.cached("RSET")); + public static final SmtpCommand EXPN = new SmtpCommand(AsciiString.cached("EXPN")); + public static final SmtpCommand VRFY = new SmtpCommand(AsciiString.cached("VRFY")); + public static final SmtpCommand HELP = new SmtpCommand(AsciiString.cached("HELP")); + public static final SmtpCommand QUIT = new SmtpCommand(AsciiString.cached("QUIT")); - private static final CharSequence DATA_CMD = AsciiString.cached("DATA"); - private static final Map COMMANDS = new HashMap(); + private static final Map COMMANDS = new HashMap(); static { - COMMANDS.put(EHLO.name(), EHLO); - COMMANDS.put(HELO.name(), HELO); - COMMANDS.put(MAIL.name(), MAIL); - COMMANDS.put(RCPT.name(), RCPT); - COMMANDS.put(DATA.name(), DATA); - COMMANDS.put(NOOP.name(), NOOP); - COMMANDS.put(RSET.name(), RSET); - COMMANDS.put(EXPN.name(), EXPN); - COMMANDS.put(VRFY.name(), VRFY); - COMMANDS.put(HELP.name(), HELP); - COMMANDS.put(QUIT.name(), QUIT); + COMMANDS.put(EHLO.name().toString(), EHLO); + COMMANDS.put(HELO.name().toString(), HELO); + COMMANDS.put(MAIL.name().toString(), MAIL); + COMMANDS.put(RCPT.name().toString(), RCPT); + COMMANDS.put(DATA.name().toString(), DATA); + COMMANDS.put(NOOP.name().toString(), NOOP); + COMMANDS.put(RSET.name().toString(), RSET); + COMMANDS.put(EXPN.name().toString(), EXPN); + COMMANDS.put(VRFY.name().toString(), VRFY); + COMMANDS.put(HELP.name().toString(), HELP); + COMMANDS.put(QUIT.name().toString(), QUIT); } /** * Returns the {@link SmtpCommand} for the given command name. */ public static SmtpCommand valueOf(CharSequence commandName) { - SmtpCommand command = COMMANDS.get(commandName); - if (command != null) { - return command; - } - return new SmtpCommand(AsciiString.of(ObjectUtil.checkNotNull(commandName, "commandName")), - AsciiString.contentEqualsIgnoreCase(commandName, DATA_CMD)); + ObjectUtil.checkNotNull(commandName, "commandName"); + SmtpCommand command = COMMANDS.get(commandName.toString()); + return command != null ? command : new SmtpCommand(AsciiString.of(commandName)); } private final AsciiString name; - private final boolean contentExpected; - private int hashCode; - private SmtpCommand(AsciiString name, boolean contentExpected) { + private SmtpCommand(AsciiString name) { this.name = name; - this.contentExpected = contentExpected; } /** @@ -86,19 +79,16 @@ public final class SmtpCommand { } void encode(ByteBuf buffer) { - ByteBufUtil.writeAscii(buffer, name()); + ByteBufUtil.writeAscii(buffer, name); } boolean isContentExpected() { - return contentExpected; + return this.equals(DATA); } @Override public int hashCode() { - if (hashCode != -1) { - hashCode = AsciiString.hashCode(name); - } - return hashCode; + return name.hashCode(); } @Override @@ -114,10 +104,6 @@ public final class SmtpCommand { @Override public String toString() { - return "SmtpCommand{" + - "name=" + name + - ", contentExpected=" + contentExpected + - ", hashCode=" + hashCode + - '}'; + return "SmtpCommand{name=" + name + '}'; } } diff --git a/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpCommandTest.java b/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpCommandTest.java new file mode 100644 index 0000000000..a5601c1627 --- /dev/null +++ b/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpCommandTest.java @@ -0,0 +1,45 @@ +/* + * Copyright 2017 The Netty Project + * + * The Netty Project 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 io.netty.handler.codec.smtp; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class SmtpCommandTest { + @Test + public void getCommandFromCache() { + assertSame(SmtpCommand.DATA, SmtpCommand.valueOf("DATA")); + assertSame(SmtpCommand.EHLO, SmtpCommand.valueOf("EHLO")); + assertNotSame(SmtpCommand.EHLO, SmtpCommand.valueOf("ehlo")); + } + + @Test + public void equalsIgnoreCase() { + assertEquals(SmtpCommand.MAIL, SmtpCommand.valueOf("mail")); + assertEquals(SmtpCommand.valueOf("test"), SmtpCommand.valueOf("TEST")); + } + + @Test + public void isContentExpected() { + assertTrue(SmtpCommand.valueOf("DATA").isContentExpected()); + assertTrue(SmtpCommand.valueOf("data").isContentExpected()); + + assertFalse(SmtpCommand.HELO.isContentExpected()); + assertFalse(SmtpCommand.HELP.isContentExpected()); + assertFalse(SmtpCommand.valueOf("DATA2").isContentExpected()); + } +}