[
https://issues.apache.org/jira/browse/SHINDIG-1647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13134417#comment-13134417
]
[email protected] commented on SHINDIG-1647:
--------------------------------------------------------
bq. On 2011-10-21 19:34:35, Jesse Ciancetta wrote:
bq. >
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/config/ValueProvider.java,
line 219
bq. > <https://reviews.apache.org/r/2467/diff/3/?file=52114#file52114line219>
bq. >
bq. > I'm guessing this is supposed to be:
bq. >
bq. > cache.putIfAbsent(container, Maps.newConcurrentMap());
bq. >
bq. > ?
bq. >
bq. > Or maybe this is the way it was intended which (I think) would make
the next line not needed?
bq. >
bq. > I think I'd still do it in two lines either way though.
bq.
bq. Ryan Baxter wrote:
bq. Jesse see my comment above and Dan's response.
Thanks Ryan. Dan's previous comment helps to make it more clear, but the
assignment to oldValues inside of the call to putIfAbsent(...) still doesn't
look quite right/needed to me. I think just removing that assignment and
adding a comment inside the if block would clear things up:
ConcurrentMap<String, Object> oldValues = cache.get(container);
if (oldValues == null) {
//There wasnt a value in the cache yet when we checked a couple of
lines ago, but there could
//be by now if another thread already executed the code below us... So
to be safe we'll call
//putIfAbsent(...) and then go ahead and fetch the value again -- this
should keep everything
//thread safe... See the docs on putIfAbsent(...) for more detail.
cache.putIfAbsent(container, Maps.<String, Object>newConcurrentMap());
oldValues = cache.get(container);
}
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2467/#review2753
-----------------------------------------------------------
On 2011-10-24 16:28:55, Stanton Sievers wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2467/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-24 16:28:55)
bq.
bq.
bq. Review request for Ryan Baxter, Dan Dumont and Jesse Ciancetta.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This patch adds an abstraction layer between implementations and
configuration. The patch consists of an abstract class and some interfaces
that are used to read and observe ContainerConfig. Implementors of this class
could decide to read their configuration from ContainerConfig or provide values
from another source. This also allows code that needs to use configuration to
not have to worry about managing container.js keys. They can simply ask their
provider for values.
bq.
bq. I've separated this out from another review as it is generic. To see an
implementation of ValueProvider, you can look at this review:
https://reviews.apache.org/r/2362
bq.
bq.
bq. This addresses bug SHINDIG-1647.
bq. https://issues.apache.org/jira/browse/SHINDIG-1647
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/config/ValueProvider.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/2467/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. No specific testing done on the patch, but testing was done in the other
review for those classes that implement and use it.
bq.
bq.
bq. Thanks,
bq.
bq. Stanton
bq.
bq.
> Provide an abstraction between implementations and the configurations they use
> ------------------------------------------------------------------------------
>
> Key: SHINDIG-1647
> URL: https://issues.apache.org/jira/browse/SHINDIG-1647
> Project: Shindig
> Issue Type: New Feature
> Components: Java
> Affects Versions: 3.0.0
> Reporter: Stanton Sievers
> Fix For: 3.0.0
>
>
> Add an abstraction layer between implementations and configuration. The
> patch consists of an abstract class and some interfaces that are used to read
> and observe ContainerConfig. Implementors of this class could decide to read
> their configuration from ContainerConfig or provide values from another
> source. This also allows code that needs to use configuration to not have to
> worry about managing container.js keys. They can simply ask their provider
> for values.
> I've separated this out from another review as it is generic. To see an
> implementation of ValueProvider, you can look at this review:
> https://reviews.apache.org/r/2362
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
