Skip to content

Commit

Permalink
Fix the commands cache and hashCode() in SmtpCommand
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fenik17 authored and normanmaurer committed Aug 18, 2017
1 parent 03d89c2 commit 0234878
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,53 +29,46 @@
*/
@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<CharSequence, SmtpCommand> COMMANDS = new HashMap<CharSequence, SmtpCommand>();
private static final Map<String, SmtpCommand> COMMANDS = new HashMap<String, SmtpCommand>();
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;
}

/**
Expand All @@ -86,19 +79,16 @@ public AsciiString name() {
}

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
Expand All @@ -114,10 +104,6 @@ public boolean equals(Object obj) {

@Override
public String toString() {
return "SmtpCommand{" +
"name=" + name +
", contentExpected=" + contentExpected +
", hashCode=" + hashCode +
'}';
return "SmtpCommand{name=" + name + '}';
}
}
Original file line number Diff line number Diff line change
@@ -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());
}
}

0 comments on commit 0234878

Please sign in to comment.