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

Issue 12523046: basic forms authentication support for crawling (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by Tanmay Vartak
Modified:
10 years, 7 months ago
Reviewers:
ejona
CC:
connector-cr_google.com
Visibility:
Public.

Description

basic forms authentication support for crawling

Patch Set 1 #

Patch Set 2 : with new files #

Total comments: 38

Patch Set 3 : with code review comments and test integration #

Total comments: 17

Patch Set 4 : with code review comments #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+136 lines, -56 lines) Patch
M build.xml View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M src/com/google/enterprise/adaptor/sharepoint/FormsAuthenticationHandler.java View 1 2 3 5 chunks +22 lines, -15 lines 1 comment Download
src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java View 1 2 3 8 chunks +14 lines, -8 lines 0 comments Download
src/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptor.java View 1 2 3 7 chunks +14 lines, -13 lines 2 comments Download
M test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java View 1 2 3 18 chunks +80 lines, -14 lines 0 comments Download
test/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptorTest.java View 1 2 3 2 chunks +4 lines, -4 lines 1 comment Download

Messages

Total messages: 9
Tanmay Vartak
10 years, 7 months ago (2013-08-08 23:12:11 UTC) #1
Tanmay Vartak
10 years, 7 months ago (2013-08-08 23:12:59 UTC) #2
Tanmay Vartak
this is not the final CL. Need to include unit tests https://codereview.appspot.com/12523046/diff/4001/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java File src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java (right): ...
10 years, 7 months ago (2013-08-08 23:17:35 UTC) #3
ejona
https://codereview.appspot.com/12523046/diff/4001/build.xml File build.xml (right): https://codereview.appspot.com/12523046/diff/4001/build.xml#newcode170 build.xml:170: <copy file="Authentication.wsdl" Put this after UserGroup, so we have ...
10 years, 7 months ago (2013-08-09 21:35:47 UTC) #4
Tanmay Vartak
10 years, 7 months ago (2013-08-12 22:08:51 UTC) #5
Tanmay Vartak
I just added mock authentication support so that existing tests are not broken. These tests ...
10 years, 7 months ago (2013-08-12 22:14:26 UTC) #6
ejona
https://codereview.appspot.com/12523046/diff/12001/build.xml File build.xml (right): https://codereview.appspot.com/12523046/diff/12001/build.xml#newcode163 build.xml:163: <arg value="com.microsoft.schemas.sharepoint.soap.Authentication"/> Can we have the package name be ...
10 years, 7 months ago (2013-08-15 21:31:05 UTC) #7
Tanmay Vartak
10 years, 7 months ago (2013-08-16 23:17:12 UTC) #8
ejona
10 years, 7 months ago (2013-08-17 21:57:58 UTC) #9
LGTM

https://codereview.appspot.com/12523046/diff/26001/src/com/google/enterprise/...
File
src/com/google/enterprise/adaptor/sharepoint/FormsAuthenticationHandler.java
(right):

https://codereview.appspot.com/12523046/diff/26001/src/com/google/enterprise/...
src/com/google/enterprise/adaptor/sharepoint/FormsAuthenticationHandler.java:101:
+ "web service with Error Code" + result.getErrorCode());
Add a space after 'Code' so that there is a space between it and the error code.

https://codereview.appspot.com/12523046/diff/26001/src/com/google/enterprise/...
File
src/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptor.java
(right):

https://codereview.appspot.com/12523046/diff/26001/src/com/google/enterprise/...
src/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptor.java:295:
if (!cookies.isEmpty()) {
Similar comment as elsewhere.

https://codereview.appspot.com/12523046/diff/26001/src/com/google/enterprise/...
src/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptor.java:299:
(BindingProvider) userProfileChangeServiceSoap, cookies);
Indent two more.

https://codereview.appspot.com/12523046/diff/26001/test/com/google/enterprise...
File
test/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptorTest.java
(right):

https://codereview.appspot.com/12523046/diff/26001/test/com/google/enterprise...
test/com/google/enterprise/adaptor/sharepoint/SharePointUserProfileAdaptorTest.java:3:
import com.google.enterprise.adaptor.Acl;
This should go with the other adaptor imports.
Sign in to reply to this message.

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