Skip to content

Support method-chain syntax to create packer and unpacker with immutable config #325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xerial merged 4 commits into from
Jan 7, 2016
Merged

Support method-chain syntax to create packer and unpacker with immutable config #325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5039f16
Support method-chain syntax to create packer and unpacker with config
frsyuki Jan 5, 2016
e34df59
Make buffer size of StreamBuffer{Input,Output} and ChannelBuffer{Inpu…
frsyuki Jan 5, 2016
61184bf
make PackerConfg and UnpackerConfig immutable to make
frsyuki Jan 6, 2016
b1e30b2
added test code for PackerConfig and UnpackerConfig
frsyuki Jan 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
273 changes: 235 additions & 38 deletions msgpack-core/src/main/java/org/msgpack/core/MessagePack.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,41 @@ public static MessageUnpacker newDefaultUnpacker(byte[] contents, int offset, in
* MessagePacker configuration.
*/
public static class PackerConfig
implements Cloneable
{
/**
* Use String.getBytes() for converting Java Strings that are smaller than this threshold into UTF8.
* Note that this parameter is subject to change.
*/
public int smallStringOptimizationThreshold = 512;
private int smallStringOptimizationThreshold = 512;

/**
* When the next payload size exceeds this threshold, MessagePacker will call MessageBufferOutput.flush() before
* packing the data.
*/
public int bufferFlushThreshold = 8192;
private int bufferFlushThreshold = 8192;

private int bufferSize = 8192;

public PackerConfig()
{ }

private PackerConfig(PackerConfig copy)
{
this.smallStringOptimizationThreshold = smallStringOptimizationThreshold;
this.bufferFlushThreshold = bufferFlushThreshold;
this.bufferSize = bufferSize;
}

@Override
public PackerConfig clone()
{
return new PackerConfig(this);
}

@Override
public boolean equals(Object obj)
{
if (!(obj instanceof PackerConfig)) {
return false;
}
PackerConfig o = (PackerConfig) obj;
return this.smallStringOptimizationThreshold == o.smallStringOptimizationThreshold
&& this.bufferFlushThreshold == o.bufferFlushThreshold
&& this.bufferSize == o.bufferSize;
}

/**
* Create a packer that outputs the packed data to a given output
Expand All @@ -266,7 +289,7 @@ public MessagePacker newPacker(MessageBufferOutput out)
*/
public MessagePacker newPacker(OutputStream out)
{
return newPacker(new OutputStreamBufferOutput(out));
return newPacker(new OutputStreamBufferOutput(out, bufferSize));
}

/**
Expand All @@ -277,7 +300,7 @@ public MessagePacker newPacker(OutputStream out)
*/
public MessagePacker newPacker(WritableByteChannel channel)
{
return newPacker(new ChannelBufferOutput(channel));
return newPacker(new ChannelBufferOutput(channel, bufferSize));
}

/**
Expand All @@ -289,42 +312,110 @@ public MessageBufferPacker newBufferPacker()
{
return new MessageBufferPacker(this);
}
}

/**
* MessageUnpacker configuration.
*/
public static class UnpackerConfig
{
/**
* Allow unpackBinaryHeader to read str format family (default:true)
* Use String.getBytes() for converting Java Strings that are smaller than this threshold into UTF8.
* Note that this parameter is subject to change.
*/
public boolean allowReadingStringAsBinary = true;
public PackerConfig withSmallStringOptimizationThreshold(int bytes)
{
PackerConfig copy = clone();
copy.smallStringOptimizationThreshold = bytes;
return copy;
}

/**
* Allow unpackRawStringHeader and unpackString to read bin format family (default: true)
*/
public boolean allowReadingBinaryAsString = true;
public int getSmallStringOptimizationThreshold()
{
return smallStringOptimizationThreshold;
}

/**
* Action when encountered a malformed input
* When the next payload size exceeds this threshold, MessagePacker will call MessageBufferOutput.flush() before
* packing the data (default: 8192).
*/
public CodingErrorAction actionOnMalformedString = CodingErrorAction.REPLACE;
public PackerConfig withBufferFlushThreshold(int bytes)
{
PackerConfig copy = clone();
copy.bufferFlushThreshold = bytes;
return copy;
}

/**
* Action when an unmappable character is found
*/
public CodingErrorAction actionOnUnmappableString = CodingErrorAction.REPLACE;
public int getBufferFlushThreshold()
{
return bufferFlushThreshold;
}

/**
* unpackString size limit. (default: Integer.MAX_VALUE)
* When a packer is created with newPacker(OutputStream) or newPacker(WritableByteChannel), the stream will be
* buffered with this size of buffer (default: 8192).
*/
public int stringSizeLimit = Integer.MAX_VALUE;
public PackerConfig withBufferSize(int bytes)
{
PackerConfig copy = clone();
copy.bufferSize = bytes;
return copy;
}

/**
*
*/
public int stringDecoderBufferSize = 8192;
public int getBufferSize()
{
return bufferSize;
}
}

/**
* MessageUnpacker configuration.
*/
public static class UnpackerConfig
implements Cloneable
{
private boolean allowReadingStringAsBinary = true;
Copy link
Member

@xerial xerial Jan 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that Packer/UnpackerConfig objects are immutable, we can make them public final to remove redundant getter methods. What do you think?

Copy link
Member Author

@frsyuki frsyuki Jan 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using getter is safer here because {Packer,Unpacker}Config are public API and change of the API means backward incompatibility. Exposing internal data structure seems negative for the future development.

Copy link
Member

@xerial xerial Jan 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we may need to use @deprecated annotation for the future config change. OK.


private boolean allowReadingBinaryAsString = true;

private CodingErrorAction actionOnMalformedString = CodingErrorAction.REPLACE;

private CodingErrorAction actionOnUnmappableString = CodingErrorAction.REPLACE;

private int stringSizeLimit = Integer.MAX_VALUE;

private int bufferSize = 8192;

private int stringDecoderBufferSize = 8192;

public UnpackerConfig()
{ }

private UnpackerConfig(UnpackerConfig copy)
{
this.allowReadingStringAsBinary = copy.allowReadingStringAsBinary;
this.allowReadingBinaryAsString = copy.allowReadingBinaryAsString;
this.actionOnMalformedString = copy.actionOnMalformedString;
this.actionOnUnmappableString = copy.actionOnUnmappableString;
this.stringSizeLimit = copy.stringSizeLimit;
this.bufferSize = copy.bufferSize;
}

@Override
public UnpackerConfig clone()
{
return new UnpackerConfig(this);
}

@Override
public boolean equals(Object obj)
Copy link
Member

@xerial xerial Jan 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test code to check the equality of config objects

Copy link
Member Author

@frsyuki frsyuki Jan 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

{
if (!(obj instanceof UnpackerConfig)) {
return false;
}
UnpackerConfig o = (UnpackerConfig) obj;
return this.allowReadingStringAsBinary == o.allowReadingStringAsBinary
&& this.allowReadingBinaryAsString == o.allowReadingBinaryAsString
&& this.actionOnMalformedString == o.actionOnMalformedString
&& this.actionOnUnmappableString == o.actionOnUnmappableString
&& this.stringSizeLimit == o.stringSizeLimit
&& this.stringDecoderBufferSize == o.stringDecoderBufferSize
&& this.bufferSize == o.bufferSize;
}

/**
* Create an unpacker that reads the data from a given input
Expand All @@ -345,7 +436,7 @@ public MessageUnpacker newUnpacker(MessageBufferInput in)
*/
public MessageUnpacker newUnpacker(InputStream in)
{
return newUnpacker(new InputStreamBufferInput(in));
return newUnpacker(new InputStreamBufferInput(in, bufferSize));
}

/**
Expand All @@ -356,7 +447,7 @@ public MessageUnpacker newUnpacker(InputStream in)
*/
public MessageUnpacker newUnpacker(ReadableByteChannel channel)
{
return newUnpacker(new ChannelBufferInput(channel));
return newUnpacker(new ChannelBufferInput(channel, bufferSize));
}

/**
Expand All @@ -380,5 +471,111 @@ public MessageUnpacker newUnpacker(byte[] contents, int offset, int length)
{
return newUnpacker(new ArrayBufferInput(contents, offset, length));
}

/**
* Allow unpackBinaryHeader to read str format family (default: true)
*/
public UnpackerConfig withAllowReadingStringAsBinary(boolean enable)
{
UnpackerConfig copy = clone();
copy.allowReadingStringAsBinary = enable;
return copy;
}

public boolean getAllowReadingStringAsBinary()
{
return allowReadingStringAsBinary;
}

/**
* Allow unpackString and unpackRawStringHeader and unpackString to read bin format family (default: true)
*/
public UnpackerConfig withAllowReadingBinaryAsString(boolean enable)
{
UnpackerConfig copy = clone();
copy.allowReadingBinaryAsString = enable;
return copy;
}

public boolean getAllowReadingBinaryAsString()
{
return allowReadingBinaryAsString;
}

/**
* Action when encountered a malformed input (default: REPLACE)
*/
public UnpackerConfig withActionOnMalformedString(CodingErrorAction action)
{
UnpackerConfig copy = clone();
copy.actionOnMalformedString = action;
return copy;
}

public CodingErrorAction getActionOnMalformedString()
{
return actionOnMalformedString;
}

/**
* Action when an unmappable character is found (default: REPLACE)
*/
public UnpackerConfig withActionOnUnmappableString(CodingErrorAction action)
{
UnpackerConfig copy = clone();
copy.actionOnUnmappableString = action;
return copy;
}

public CodingErrorAction getActionOnUnmappableString()
{
return actionOnUnmappableString;
}

/**
* unpackString size limit (default: Integer.MAX_VALUE).
*/
public UnpackerConfig withStringSizeLimit(int bytes)
{
UnpackerConfig copy = clone();
copy.stringSizeLimit = bytes;
return copy;
}

public int getStringSizeLimit()
{
return stringSizeLimit;
}

/**
*
*/
public UnpackerConfig withStringDecoderBufferSize(int bytes)
{
UnpackerConfig copy = clone();
copy.stringDecoderBufferSize = bytes;
return copy;
}

public int getStringDecoderBufferSize()
{
return stringDecoderBufferSize;
}

/**
* When a packer is created with newUnpacker(OutputStream) or newUnpacker(WritableByteChannel), the stream will be
* buffered with this size of buffer (default: 8192).
*/
public UnpackerConfig withBufferSize(int bytes)
{
UnpackerConfig copy = clone();
copy.bufferSize = bytes;
return copy;
}

public int getBufferSize()
{
return bufferSize;
}
}
}
5 changes: 2 additions & 3 deletions msgpack-core/src/main/java/org/msgpack/core/MessagePacker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,8 @@ public class MessagePacker
public MessagePacker(MessageBufferOutput out, MessagePack.PackerConfig config)
{
this.out = checkNotNull(out, "MessageBufferOutput is null");
// We must copy the configuration parameters here since the config object is mutable
this.smallStringOptimizationThreshold = config.smallStringOptimizationThreshold;
this.bufferFlushThreshold = config.bufferFlushThreshold;
this.smallStringOptimizationThreshold = config.getSmallStringOptimizationThreshold();
this.bufferFlushThreshold = config.getBufferFlushThreshold();
this.position = 0;
this.totalFlushBytes = 0;
}
Expand Down
13 changes: 6 additions & 7 deletions msgpack-core/src/main/java/org/msgpack/core/MessageUnpacker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,12 @@ public class MessageUnpacker
public MessageUnpacker(MessageBufferInput in, MessagePack.UnpackerConfig config)
{
this.in = checkNotNull(in, "MessageBufferInput is null");
// We need to copy the configuration parameters since the config object is mutable
this.allowReadingStringAsBinary = config.allowReadingStringAsBinary;
this.allowReadingBinaryAsString = config.allowReadingBinaryAsString;
this.actionOnMalformedString = config.actionOnMalformedString;
this.actionOnUnmappableString = config.actionOnUnmappableString;
this.stringSizeLimit = config.stringSizeLimit;
this.stringDecoderBufferSize = config.stringDecoderBufferSize;
this.allowReadingStringAsBinary = config.getAllowReadingStringAsBinary();
this.allowReadingBinaryAsString = config.getAllowReadingBinaryAsString();
this.actionOnMalformedString = config.getActionOnMalformedString();
this.actionOnUnmappableString = config.getActionOnUnmappableString();
this.stringSizeLimit = config.getStringSizeLimit();
this.stringDecoderBufferSize = config.getStringDecoderBufferSize();
}

/**
Expand Down
13 changes: 6 additions & 7 deletions msgpack-core/src/test/java/org/msgpack/core/example/MessagePackExample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,20 +246,19 @@ public static void configuration()
throws IOException
{
ByteArrayOutputStream out = new ByteArrayOutputStream();
PackerConfig packerConfig = new PackerConfig();
packerConfig.smallStringOptimizationThreshold = 256; // String
MessagePacker packer = packerConfig.newPacker(out);
MessagePacker packer = new PackerConfig()
.withSmallStringOptimizationThreshold(256) // String
.newPacker(out);

packer.packInt(10);
packer.packBoolean(true);
packer.close();

// Unpack data
UnpackerConfig unpackerConfig = new UnpackerConfig();
unpackerConfig.stringDecoderBufferSize = 16 * 1024; // If your data contains many large strings (the default is 8k)

byte[] packedData = out.toByteArray();
MessageUnpacker unpacker = unpackerConfig.newUnpacker(packedData);
MessageUnpacker unpacker = new UnpackerConfig()
.withStringDecoderBufferSize(16 * 1024) // If your data contains many large strings (the default is 8k)
.newUnpacker(packedData);
int i = unpacker.unpackInt(); // 10
boolean b = unpacker.unpackBoolean(); // true
unpacker.close();
Expand Down
Loading