----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2648/#review2993 -----------------------------------------------------------
Ship it! This looks good to me, however if we're going to go this route I wouldn't mind seeing the BlobCrypterSecurityTokenCodec changed to always expect to get the actual token key from ContainerConfig -- and then making the code that parses the container.js file smart enough to be able to resolve res://key-file or file:///path/to/key-file automatically (or if not prefixed to take the value from container.js as is). I think this would work well for this particular case and would be a nice addition to the ContainerConfig code. It's pretty straight forward to add that support to ContainerConfig and I've got a patch that does that which I could post if people are interested -- I'd need to add some additional unit tests around the enhancements to the ContainerConfig loader but other than that it should be good to go. Or you could commit this as is and I could post that as a follow on patch at a later date and if people like it we could move to it. Either way is fine with me. - Jesse On 2011-11-01 18:12:28, Stanton Sievers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2648/ > ----------------------------------------------------------- > > (Updated 2011-11-01 18:12:28) > > > Review request for shindig, Ryan Baxter and Dan Dumont. > > > Summary > ------- > > This patch is to illustrate the minimal set of changes needed to allow the > BlobCrypterSecurityTokenCodec to have a key provided by the config instead of > needing a key file to be provided. > > > This addresses bug SHINDIG-1636. > https://issues.apache.org/jira/browse/SHINDIG-1636 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java > 1195457 > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1195457 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java > 1195457 > > Diff: https://reviews.apache.org/r/2648/diff > > > Testing > ------- > > Updated existing JUnits and added one more. > > > Thanks, > > Stanton > >