-
Notifications
You must be signed in to change notification settings - Fork 156
READY: Settable Default Charset #550
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.
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
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
dcacf25
First pass at client-settable default charset
alexmoore 8ef9eeb
Logging charset changes
alexmoore 75367b7
Adding javadocs
alexmoore a75a855
Removing unnecessary code
alexmoore 1e28ea7
Changing docs to reflect that we're only classloader-wide singletons
alexmoore 9474a5b
Adding more descriptive logging
alexmoore 68f314a
adding a few initializer tests
alexmoore 1f0b871
Reset our properties correctly in the test...
alexmoore 3942e49
Cleanup the DefaultCharsetTest
alexmoore File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
115 changes: 115 additions & 0 deletions
115
src/main/java/com/basho/riak/client/core/util/DefaultCharset.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| package com.basho.riak.client.core.util; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.nio.charset.Charset; | ||
| import java.util.concurrent.atomic.AtomicReference; | ||
|
|
||
| /** | ||
| * <p> | ||
| * Holds an classloader-wide default charset, that | ||
| * is then used to encode/decode between Strings and | ||
| * Byte arrays for Riak's use (see {@link com.basho.riak.client.core.util.BinaryValue}). | ||
| * </p> | ||
| * <p/> | ||
| * <p> | ||
| * Before 2.0.3, the system used the JRE's default Charset from the | ||
| * {@link Charset#defaultCharset()} property. | ||
| * </p> | ||
| * <p/> | ||
| * <p> | ||
| * With this class you may change the Riak client to use a different default. | ||
| * You can set it at startup by providing the desired Charset name with the vm argument | ||
| * {@code -Dcom.basho.riak.client.DefaultCharset="UTF-8"}, or at runtime | ||
| * with the static method (see {@link #set(Charset)}). | ||
| * </p> | ||
| * <p/> | ||
| * <p> | ||
| * As of 2.0.3 it still defaults to the value provided by | ||
| * {@link Charset#defaultCharset()}, | ||
| * but the default is planned to change to "UTF-8" with 2.1.0. | ||
| * </p> | ||
| * <p/> | ||
| * <p> | ||
| * If your JRE default charset is one of "<b>US-ASCII</b>", | ||
| * "<b>UTF-8</b>", or "<b>ISO-8859-1</b>", | ||
| * this change should <b>not</b> affect you. | ||
| * </p> | ||
| * <p/> | ||
| * <p> | ||
| * If your JRE default charset is one of "<b>UTF-16</b>", | ||
| * "<b>UTF-16BE</b>", or "<b>UTF-16LE</b>", | ||
| * <b>you will need to set that default on the command line or | ||
| * after application startup once you upgrade</b>. | ||
| * </p> | ||
| * | ||
| * @author Alex Moore <amoore AT basho dot com> | ||
| * @since 2.0.3 | ||
| */ | ||
|
|
||
| public final class DefaultCharset | ||
| { | ||
| private static final Logger logger = LoggerFactory.getLogger(DefaultCharset.class); | ||
| private final static DefaultCharset instance = initializeDefaultCharsetSingleton(); | ||
|
|
||
| private final AtomicReference<Charset> currentCharset; | ||
|
|
||
| private DefaultCharset(Charset c) | ||
| { | ||
| logger.info("Initializing client charset to: {}", c.name()); | ||
| this.currentCharset = new AtomicReference<Charset>(c); | ||
| } | ||
|
|
||
| private static DefaultCharset initializeDefaultCharsetSingleton() | ||
| { | ||
| Charset charset; | ||
| final Charset systemDefault = Charset.defaultCharset(); | ||
| final String declaredCharsetName = System.getProperty(Constants.CLIENT_OPTION_CHARSET); | ||
|
|
||
| if (declaredCharsetName != null && !declaredCharsetName.isEmpty()) | ||
| { | ||
| try | ||
| { | ||
| charset = Charset.forName(declaredCharsetName); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| charset = systemDefault; | ||
| logger.warn("Requested charset '{}' is not available, the default charset '{}' will be used", | ||
| declaredCharsetName, | ||
| charset.name()); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| logger.info("No desired charset found in system properties, the default charset '{}' will be used", | ||
| systemDefault.name()); | ||
| charset = systemDefault; | ||
| } | ||
|
|
||
| return new DefaultCharset(charset); | ||
| } | ||
|
|
||
| /** | ||
| * Get the current classloader-wide default Charset for the Riak client. | ||
| * | ||
| * @return The current classloader-wide default charset. | ||
| */ | ||
| public static Charset get() | ||
| { | ||
| return instance.currentCharset.get(); | ||
| } | ||
|
|
||
| /** | ||
| * Set the classloader-wide default Charset for the Riak client. | ||
| * | ||
| * @param charset The charset to set the classloader-wide default to. | ||
| */ | ||
| public static void set(Charset charset) | ||
| { | ||
| final Charset current = instance.currentCharset.get(); | ||
| logger.info("Setting client charset from '{}' to '{}'", current.name(), charset.name()); | ||
| instance.currentCharset.set(charset); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know the specifics of JVM instantiation vs C#, but is this guaranteed to be thread safe? Do we care?
In C# I prefer the nested class approach and it appears to be a good way of doing it in Java too:
http://literatejava.com/jvm/fastest-threadsafe-singleton-jvm/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most simple way, but not the most obvious one, is to declare it as an enum https://dzone.com/articles/singleton-design-pattern-–
THere is also interesting academic case with hacking the Singleton... For the enum approach there is no chance to hack it with Reflection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is both interesting and gross :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static initializer should only run once, the JVM uses a lock the first time a class is loaded: http://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.4.2. Also it should be initialized the first time the
get()method is called: http://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.4.1.The AtomicReference should take care of any other cross-threading fun.