From 76c102b096c36fe977bcd5cb9742fcee140557cd Mon Sep 17 00:00:00 2001 From: Roman Proshin Date: Wed, 17 Jan 2018 23:46:42 +0300 Subject: [PATCH] #38: Refactor the Manifests class to an immutable --- .../java/com/jcabi/manifests/Manifests.java | 83 +++++++++---------- src/main/java/com/jcabi/manifests/MfMap.java | 61 +++++++++++++- .../com/jcabi/manifests/ManifestsTest.java | 12 ++- 3 files changed, 101 insertions(+), 55 deletions(-) diff --git a/src/main/java/com/jcabi/manifests/Manifests.java b/src/main/java/com/jcabi/manifests/Manifests.java index fb9cff8..82a9d05 100644 --- a/src/main/java/com/jcabi/manifests/Manifests.java +++ b/src/main/java/com/jcabi/manifests/Manifests.java @@ -35,11 +35,13 @@ import java.io.InputStream; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicReference; import java.util.jar.Attributes; import java.util.jar.Manifest; import javax.servlet.ServletContext; @@ -126,7 +128,8 @@ public final class Manifests implements MfMap { /** * Default singleton. */ - public static final MfMap DEFAULT = new Manifests(); + private static final AtomicReference DEFAULT = + new AtomicReference(new Manifests()); /** * Attributes retrieved. @@ -135,7 +138,11 @@ public final class Manifests implements MfMap { static { try { - Manifests.DEFAULT.append(new ClasspathMfs()); + final MfMap currentInstance = Manifests.DEFAULT.get(); + Manifests.DEFAULT.compareAndSet( + currentInstance, + currentInstance.append(new ClasspathMfs()) + ); } catch (final IOException ex) { Logger.error( Manifests.class, @@ -173,57 +180,34 @@ public boolean isEmpty() { } @Override - public boolean containsKey(final Object key) { + public boolean containsKey(final String key) { return this.attributes.containsKey(key); } @Override - public boolean containsValue(final Object value) { + public boolean containsValue(final String value) { return this.attributes.containsValue(value); } @Override - public String get(final Object key) { + public String get(final String key) { return this.attributes.get(key); } @Override - public String put(final String key, final String value) { - return this.attributes.put(key, value); - } - - @Override - public String remove(final Object key) { - return this.attributes.remove(key); - } - - @Override - public void putAll(final Map attrs) { - this.attributes.putAll(attrs); - } - - @Override - public void clear() { - this.attributes.clear(); + public Map getAsMap() { + return new HashMap(this.attributes); } @Override public Set keySet() { - return this.attributes.keySet(); - } - - @Override - public Collection values() { - return this.attributes.values(); - } - - @Override - public Set> entrySet() { - return this.attributes.entrySet(); + return new HashSet(this.attributes.keySet()); } @Override public MfMap append(final Mfs streams) throws IOException { + final ConcurrentHashMap joinedAttributes = + new ConcurrentHashMap(this.attributes); final long start = System.currentTimeMillis(); final Collection list = streams.fetch(); int saved = 0; @@ -231,10 +215,10 @@ public MfMap append(final Mfs streams) throws IOException { for (final InputStream stream : list) { for (final Map.Entry attr : Manifests.load(stream).entrySet()) { - if (this.attributes.containsKey(attr.getKey())) { + if (joinedAttributes.containsKey(attr.getKey())) { ++ignored; } else { - this.attributes.put(attr.getKey(), attr.getValue()); + joinedAttributes.put(attr.getKey(), attr.getValue()); ++saved; } } @@ -248,7 +232,7 @@ public MfMap append(final Mfs streams) throws IOException { saved, ignored, new TreeSet(this.attributes.keySet()) ); - return this; + return new Manifests(joinedAttributes); } /** @@ -276,12 +260,12 @@ public static String read(final String name) { // @checkstyle LineLength (1 line) "Attribute '%s' not found in MANIFEST.MF file(s) among %d other attribute(s): %[list]s", name, - Manifests.DEFAULT.size(), - new TreeSet(Manifests.DEFAULT.keySet()) + Manifests.DEFAULT.get().size(), + new TreeSet(Manifests.DEFAULT.get().keySet()) ) ); } - return Manifests.DEFAULT.get(name); + return Manifests.DEFAULT.get().get(name); } /** @@ -302,7 +286,7 @@ public static boolean exists(final String name) { if (name.isEmpty()) { throw new IllegalArgumentException("attribute name can't be empty"); } - return Manifests.DEFAULT.containsKey(name); + return Manifests.DEFAULT.get().containsKey(name); } /** @@ -319,7 +303,11 @@ public static boolean exists(final String name) { */ @Deprecated public static void append(final ServletContext ctx) throws IOException { - Manifests.DEFAULT.append(new ServletMfs(ctx)); + final MfMap currentInstance = Manifests.DEFAULT.get(); + Manifests.DEFAULT.compareAndSet( + currentInstance, + currentInstance.append(new ServletMfs(ctx)) + ); } /** @@ -338,7 +326,11 @@ public static void append(final File file) throws IOException { if (file == null) { throw new IllegalArgumentException("file can't be NULL"); } - Manifests.DEFAULT.append(new FilesMfs(file)); + final MfMap currentInstance = Manifests.DEFAULT.get(); + Manifests.DEFAULT.compareAndSet( + currentInstance, + currentInstance.append(new FilesMfs(file)) + ); } /** @@ -358,7 +350,11 @@ public static void append(final InputStream stream) throws IOException { if (stream == null) { throw new IllegalArgumentException("input stream can't be NULL"); } - Manifests.DEFAULT.append(new StreamsMfs(stream)); + final MfMap currentInstance = Manifests.DEFAULT.get(); + Manifests.DEFAULT.compareAndSet( + currentInstance, + currentInstance.append(new StreamsMfs(stream)) + ); } /** @@ -403,5 +399,4 @@ private static Map load(final InputStream stream) } return props; } - } diff --git a/src/main/java/com/jcabi/manifests/MfMap.java b/src/main/java/com/jcabi/manifests/MfMap.java index c96b0e0..9191b7e 100644 --- a/src/main/java/com/jcabi/manifests/MfMap.java +++ b/src/main/java/com/jcabi/manifests/MfMap.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.util.Map; +import java.util.Set; /** * Map of manifest attributes. @@ -40,15 +41,67 @@ * @since 1.1 * @see Manifests */ -public interface MfMap extends Map { +public interface MfMap { + + /** + * Get size of attributes map. + * @return Size of attributes map + * @since 2.0 + */ + int size(); + + /** + * Check if attributes map is empty. + * @return True if attributes map is empty and false otherwise + * @since 2.0 + */ + boolean isEmpty(); + + /** + * Check if attributes map contains the given key. + * @param key Attribute name + * @return True if attributes map contains the given key and false otherwise + * @since 2.0 + */ + boolean containsKey(String key); + + /** + * Check if attributes map contains the given value. + * @param value Attribute value + * @return True if attributes map contains the given value and false otherwise + * @since 2.0 + */ + boolean containsValue(String value); + + /** + * Get attribute value by its key. + * @param key Attribute name + * @return Value of the attribute and null if attribute not found + */ + String get(String key); + + /** + * Get a copy of attributes map. + * @return Copy of attributes map + * @since 2.0 + */ + Map getAsMap(); + + /** + * Get a copy of a set of attributes keys. + * @return Copy of a set of attributes keys + * @since 2.0 + */ + Set keySet(); /** * Append this collection of MANIFEST.MF files. + * This method may not change the original instance + * and should create a new instance and return it as a result. * @param streams Files to append - * @return This - * @since 1.0 + * @return New instance that contains original and appended attributes * @throws IOException If fails on I/O problem + * @since 1.0 */ MfMap append(Mfs streams) throws IOException; - } diff --git a/src/test/java/com/jcabi/manifests/ManifestsTest.java b/src/test/java/com/jcabi/manifests/ManifestsTest.java index 5507600..76cc77a 100644 --- a/src/test/java/com/jcabi/manifests/ManifestsTest.java +++ b/src/test/java/com/jcabi/manifests/ManifestsTest.java @@ -102,11 +102,10 @@ public void appendsAttributesFromFile() throws Exception { file, Logger.format("%s: %s\n", name, value) ); - final Manifests mfs = new Manifests(); - mfs.append(new FilesMfs(file)); + final MfMap manifests = new Manifests().append(new FilesMfs(file)); MatcherAssert.assertThat( "loaded from file", - mfs.containsKey(name) && mfs.get(name).equals(value) + manifests.containsKey(name) && manifests.get(name).equals(value) ); } @@ -117,12 +116,11 @@ public void appendsAttributesFromFile() throws Exception { */ @Test public void appendsAttributesFromInputStream() throws Exception { - final Manifests mfs = new Manifests(); - mfs.append( - new StreamsMfs(this.getClass().getResourceAsStream("test.mf")) + final MfMap manifests = new Manifests().append( + new StreamsMfs(this.getClass().getResourceAsStream("test.mf")) ); MatcherAssert.assertThat( - mfs.get("From-File"), + manifests.get("From-File"), Matchers.equalTo("some test attribute") ); }