Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2386)

Issue 242760043: Support ACL updates and deletes. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 10 months ago by Srinivas
Modified:
8 years, 10 months ago
Reviewers:
brett.michael.johnson, JohnL, myk, Brett
CC:
pjo, brett.michael.johnson_gmail.com, connector-cr_google.com
Visibility:
Public.

Description

Support ACL updates and deletes. In Documentum, updates to ACL objects are recorded in dm_audittrail_acl table. Changes made to query this table for updates and deletes of ACLs.

Patch Set 1 #

Total comments: 10

Patch Set 2 : Updated SessionMock, UserMock, and GroupMock #

Total comments: 2

Patch Set 3 : Final LGTM Changes #

Total comments: 46

Patch Set 4 : Changes as per John Lacy's comments #

Total comments: 14

Patch Set 5 : Changes as per comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -118 lines) Patch
M src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java View 1 2 3 4 5 chunks +81 lines, -86 lines 0 comments Download
M test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java View 1 2 3 4 3 chunks +37 lines, -32 lines 0 comments Download

Messages

Total messages: 20
Srinivas
Please review the changes.
8 years, 10 months ago (2015-05-26 23:53:28 UTC) #1
Brett
https://codereview.appspot.com/242760043/diff/1/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/242760043/diff/1/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2777 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2777: private class SessionMock { Please take updated SessionMock, UserMock, ...
8 years, 10 months ago (2015-05-27 00:14:06 UTC) #2
Srinivas
Updated SessionMock, UserMock, and GroupMock as suggested. Please review.
8 years, 10 months ago (2015-05-27 04:41:47 UTC) #3
myk
Only a few nits on the source code; I will wait until you update the ...
8 years, 10 months ago (2015-05-27 14:39:26 UTC) #4
myk
On 2015/05/27 14:39:26, myk wrote: > Only a few nits on the source code; I ...
8 years, 10 months ago (2015-05-27 14:40:23 UTC) #5
myk
test code LGTM; please address the few nits in the source code before submitting.
8 years, 10 months ago (2015-05-27 14:49:36 UTC) #6
Brett
https://codereview.appspot.com/242760043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java File src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java (right): https://codereview.appspot.com/242760043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java#newcode657 src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java:657: logger.entering("DocumentumAdaptor", "getDocIds"); On 2015/05/27 14:39:26, myk wrote: > The ...
8 years, 10 months ago (2015-05-27 19:03:26 UTC) #7
Brett
https://codereview.appspot.com/242760043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/242760043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2743 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2743: + DocumentumAcls.DATE_FORMAT + "'), timestamp)"); Why the dateFormat conversion?
8 years, 10 months ago (2015-05-27 19:13:20 UTC) #8
Srinivas
LGTM Changes. https://codereview.appspot.com/242760043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java File src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java (right): https://codereview.appspot.com/242760043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java#newcode256 src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java:256: * can have it's own individual ACL ...
8 years, 10 months ago (2015-05-27 19:21:08 UTC) #9
Srinivas
On 2015/05/27 19:21:08, Srinivas wrote: > LGTM Changes. > > https://codereview.appspot.com/242760043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java > File src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java (right): ...
8 years, 10 months ago (2015-05-27 20:03:26 UTC) #10
JohnL
The bar for an LGTM is frustratingly low. John L https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java File src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java (right): https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java#newcode58 ...
8 years, 10 months ago (2015-05-27 20:34:50 UTC) #11
Srinivas
Please review the changes. https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java File src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java (right): https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java#newcode58 src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java:58: static final String DATE_FORMAT = ...
8 years, 10 months ago (2015-05-28 01:41:16 UTC) #12
JohnL
A bug and some TODOs. John L https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java File src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java (right): https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java#newcode307 src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java:307: String whereBoundedClauseDateOnly ...
8 years, 10 months ago (2015-05-28 03:11:53 UTC) #13
Brett
https://codereview.appspot.com/242760043/diff/40001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/242760043/diff/40001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode3012 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:3012: public void testUpdateAclsWithSameChronicleId() throws DfException, On 2015/05/28 01:41:15, Srinivas ...
8 years, 10 months ago (2015-05-28 04:57:05 UTC) #14
JohnL
Reaction to Brett's comments. John L https://codereview.appspot.com/242760043/diff/40001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/242760043/diff/40001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode3012 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:3012: public void testUpdateAclsWithSameChronicleId() ...
8 years, 10 months ago (2015-05-28 06:39:34 UTC) #15
Srinivas
Please review the changes. https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java File src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java (right): https://codereview.appspot.com/242760043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java#newcode307 src/com/google/enterprise/adaptor/documentum/DocumentumAcls.java:307: String whereBoundedClauseDateOnly = " and ...
8 years, 10 months ago (2015-05-28 22:56:24 UTC) #16
JohnL
LGTM. Please wait for Marc's review. John L
8 years, 10 months ago (2015-05-28 23:00:25 UTC) #17
myk
LGTM as well. Thank you!
8 years, 10 months ago (2015-05-28 23:47:32 UTC) #18
Brett
LGTM.
8 years, 10 months ago (2015-05-28 23:59:52 UTC) #19
Srinivas
8 years, 10 months ago (2015-05-29 01:53:25 UTC) #20
On 2015/05/28 23:59:52, Brett wrote:
> LGTM.

Committed to Documentum Adaptor
https://github.com/googlegsa/documentum/commit/535172274b71cdc8a264355b79a3d8...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b