> On June 21, 2012, 7:56 p.m., Adam Clarke wrote: > > I think in the event that there's no encrypter available we should set the > > encrypted secret to be the unencrypted secret that was passed in. The > > persistence layer should be storing the encrypted secret, so if that was > > null the information would be lost.
The setMacSecret doesn't have this behavior, though it was already checking for encrypter != null. Also, the setEncryptedXXXSecret doesn't also set the 'unencrypted' secret if there is no encrypter. So, the methods could be modified to handle the dual case, or perhaps, if there is no encrypter set as a constructor parameter, then the NoOpEncrypter should always be used and then we can eliminate the if encrypter null checks altogether. - Brian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5497/#review8462 ----------------------------------------------------------- On June 21, 2012, 6:03 p.m., Brian Lillie wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5497/ > ----------------------------------------------------------- > > (Updated June 21, 2012, 6:03 p.m.) > > > Review request for shindig. > > > Description > ------- > > Add check for non-null encrypter. There were two test methods > (testSetSecret_1 and testSetSecret_2) that were identical.. so modified > second to not supply encrypter as parameter to constructor. > > > This addresses bug SHINDIG-1810. > https://issues.apache.org/jira/browse/SHINDIG-1810 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistence.java > 1347033 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/persistence/OAuth2TokenPersistenceTest.java > 1348118 > > Diff: https://reviews.apache.org/r/5497/diff/ > > > Testing > ------- > > Tests pass > > > Thanks, > > Brian Lillie > >
