Skip to content
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

#38: Refactor the Manifests class to an immutable #43

Merged
merged 1 commit into from
Jan 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
83 changes: 39 additions & 44 deletions src/main/java/com/jcabi/manifests/Manifests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -126,7 +128,8 @@ public final class Manifests implements MfMap {
/**
* Default singleton.
*/
public static final MfMap DEFAULT = new Manifests();
private static final AtomicReference<MfMap> DEFAULT =
new AtomicReference<MfMap>(new Manifests());

/**
* Attributes retrieved.
Expand All @@ -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,
Expand Down Expand Up @@ -173,68 +180,45 @@ 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<? extends String, ? extends String> attrs) {
this.attributes.putAll(attrs);
}

@Override
public void clear() {
this.attributes.clear();
public Map<String, String> getAsMap() {
return new HashMap<String, String>(this.attributes);
}

@Override
public Set<String> keySet() {
return this.attributes.keySet();
}

@Override
public Collection<String> values() {
return this.attributes.values();
}

@Override
public Set<Map.Entry<String, String>> entrySet() {
return this.attributes.entrySet();
return new HashSet<String>(this.attributes.keySet());
}

@Override
public MfMap append(final Mfs streams) throws IOException {
final ConcurrentHashMap<String, String> joinedAttributes =
new ConcurrentHashMap<String, String>(this.attributes);
final long start = System.currentTimeMillis();
final Collection<InputStream> list = streams.fetch();
int saved = 0;
int ignored = 0;
for (final InputStream stream : list) {
for (final Map.Entry<String, String> 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;
}
}
Expand All @@ -248,7 +232,7 @@ public MfMap append(final Mfs streams) throws IOException {
saved, ignored,
new TreeSet<String>(this.attributes.keySet())
);
return this;
return new Manifests(joinedAttributes);
}

/**
Expand Down Expand Up @@ -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<String>(Manifests.DEFAULT.keySet())
Manifests.DEFAULT.get().size(),
new TreeSet<String>(Manifests.DEFAULT.get().keySet())
)
);
}
return Manifests.DEFAULT.get(name);
return Manifests.DEFAULT.get().get(name);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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))
);
}

/**
Expand All @@ -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))
);
}

/**
Expand All @@ -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))
);
}

/**
Expand Down Expand Up @@ -403,5 +399,4 @@ private static Map<String, String> load(final InputStream stream)
}
return props;
}

}
61 changes: 57 additions & 4 deletions src/main/java/com/jcabi/manifests/MfMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import java.io.IOException;
import java.util.Map;
import java.util.Set;

/**
* Map of manifest attributes.
Expand All @@ -40,15 +41,67 @@
* @since 1.1
* @see Manifests
*/
public interface MfMap extends Map<String, String> {
public interface MfMap {
Copy link
Member

Choose a reason for hiding this comment

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

@proshin-roman Maybe it's not a good idea to change the public interface of classes that are already in production? Also, what do we gain from this?


/**
* 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<String, String> getAsMap();

/**
* Get a copy of a set of attributes keys.
* @return Copy of a set of attributes keys
* @since 2.0
*/
Set<String> 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;

}
12 changes: 5 additions & 7 deletions src/test/java/com/jcabi/manifests/ManifestsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand All @@ -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")
);
}
Expand Down