Looks like the patch pretty does all the things we expect it to - and it ties nicely into the existing structure. A couple of notes: 1. You add some new REST interface calls and functions. I think that this is unnecessary. The existing /recover, /retrieve and /approve calls are sufficient. I would suggest adding a field "dataType" to the KeyRecoveryRequest. This is analogous to - and would have the same values as dataType in KeyArchivalRequest. Then, you can do the basic checks -- is the data not null, does the request exist etc. and then call your function based on the dataType. We would then store the dataType in the recovery request. For the approval request, we can read the approval type from the database record and then call the correct approval code. 2. I noticed that you called into the service functions directly, rather than using the DAO. Thats fine because we will probably want to remove the DAO in any case. 3. I'm not sure I understand your test in DRMTest. If I understand it correctly, this test will only work for your subsystem - and if I need to run the test on my subsystem, I will need to replace the cert and keyID and recompile? This is probably OK for this iteration - but we will likely want something more portable in the future junit style tests. Ade On Fri, 2013-10-25 at 14:22 -0700, Andrew Wnuk wrote: