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

Issue 317160043: Fix bug#26742691: Support for same name files in a folder - Part 3 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 3 weeks ago by Srinivas
Modified:
7 months ago
Reviewers:
JohnL
CC:
connector-cr_google.com
Visibility:
Public.

Description

Following changes are made: - Send respondNotFound for requests with DocId urls containing no object id. - If the above DocId url corresponds to a valid object, push new DocId with object id asynchronously. - Added mock implementation of AsyncDocIdPusher for tests.

Patch Set 1 #

Total comments: 20

Patch Set 2 : Changes as per review #

Total comments: 12

Patch Set 3 : Changes as per review #

Total comments: 40

Patch Set 4 : Changes as per review. #

Total comments: 29

Patch Set 5 : Changes as per review #

Total comments: 15

Patch Set 6 : Changes per review. #

Total comments: 11

Patch Set 7 : Rebase changes #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+209 lines, -17 lines) Patch
M src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java View 1 2 3 4 5 6 7 chunks +51 lines, -6 lines 1 comment Download
M test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java View 1 2 3 4 5 6 14 chunks +123 lines, -11 lines 0 comments Download
M test/com/google/enterprise/adaptor/documentum/ProxyAdaptorContext.java View 1 2 3 4 5 6 4 chunks +35 lines, -0 lines 0 comments Download

Messages

Total messages: 20
Srinivas
Please review the changes.
7 months, 3 weeks ago (2017-02-01 03:56:24 UTC) #1
JohnL
https://codereview.appspot.com/317160043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java File src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java (right): https://codereview.appspot.com/317160043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java#newcode1451 src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java:1451: if (dmPersObj != null) { You need to do ...
7 months, 3 weeks ago (2017-02-01 22:52:59 UTC) #2
Srinivas
Please review. https://codereview.appspot.com/317160043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java File src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java (right): https://codereview.appspot.com/317160043/diff/1/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java#newcode1451 src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java:1451: if (dmPersObj != null) { On 2017/02/01 ...
7 months, 2 weeks ago (2017-02-04 02:27:54 UTC) #3
JohnL
https://codereview.appspot.com/317160043/diff/1/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/1/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2029 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2029: public void testGetDocContent_404() throws Exception { On 2017/02/04 02:27:54, ...
7 months, 2 weeks ago (2017-02-07 00:59:35 UTC) #4
Srinivas
Please review the changes. https://codereview.appspot.com/317160043/diff/1/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/1/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2029 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2029: public void testGetDocContent_404() throws Exception ...
7 months, 1 week ago (2017-02-11 04:25:16 UTC) #5
JohnL
Partial review. Haven't looked at the tests yet in detail. John L https://codereview.appspot.com/317160043/diff/40001/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java File src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java ...
7 months, 1 week ago (2017-02-14 02:30:48 UTC) #6
JohnL
Finished reviewing the tests. John L https://codereview.appspot.com/317160043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2524 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2524: // First try ...
7 months, 1 week ago (2017-02-15 00:42:01 UTC) #7
Srinivas
Please review the changes. https://codereview.appspot.com/317160043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2524 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2524: // First try a DocId ...
7 months, 1 week ago (2017-02-16 05:46:12 UTC) #8
JohnL
Still looking at this. John L https://codereview.appspot.com/317160043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/20001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2524 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2524: // First try ...
7 months, 1 week ago (2017-02-17 02:07:04 UTC) #9
JohnL
More partial review. I'd still like to look closer at setParentFolderIdFromPaths. John L https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File ...
7 months, 1 week ago (2017-02-17 02:20:09 UTC) #10
JohnL
https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode1657 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:1657: return getObjectUnderTest(ProxyAdaptorContext.getInstance(), Why change this overload at all?
7 months ago (2017-02-18 01:02:22 UTC) #11
JohnL
The test gear is getting a little fragile. I've put together a followup CL to ...
7 months ago (2017-02-18 23:54:57 UTC) #12
JohnL
One last comment, perhaps. John L https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode1820 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:1820: setParentFolderIdFromPaths(dateFormat.format(lastModified), id, name, ...
7 months ago (2017-02-19 08:48:11 UTC) #13
Srinivas
Please review the changes. https://codereview.appspot.com/317160043/diff/40001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/40001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode1877 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:1877: continue; On 2017/02/17 02:06:58, JohnL ...
7 months ago (2017-02-21 01:03:02 UTC) #14
JohnL
https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2063 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2063: public void testGetDocContent_chronicleIdPartOfName() throws Exception { On 2017/02/21 01:03:01, ...
7 months ago (2017-02-21 03:18:41 UTC) #15
Srinivas
Please review the changes. https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java File test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java (right): https://codereview.appspot.com/317160043/diff/60001/test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java#newcode2063 test/com/google/enterprise/adaptor/documentum/DocumentumAdaptorTest.java:2063: public void testGetDocContent_chronicleIdPartOfName() throws Exception ...
7 months ago (2017-02-22 00:32:20 UTC) #16
JohnL
Just nits. I would, however, like to see another patch set after the rebasing, since ...
7 months ago (2017-02-22 02:38:27 UTC) #17
Srinivas
Please review the changes. https://codereview.appspot.com/317160043/diff/120001/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java File src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java (right): https://codereview.appspot.com/317160043/diff/120001/src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java#newcode42 src/com/google/enterprise/adaptor/documentum/DocumentumAdaptor.java:42: import com.documentum.fc.client.DfIdNotFoundException; On 2017/02/22 02:38:27, ...
7 months ago (2017-02-22 05:03:10 UTC) #18
JohnL
LGTM Thanks, John L
7 months ago (2017-02-22 05:31:08 UTC) #19
Srinivas
7 months ago (2017-02-22 19:41:05 UTC) #20
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted