Fix modification detection for SP 2010 We used to infinite loop when detecting modifications with SP 2010. The fix for that effectively killed modification detection outright. The real issue is that SP 2010's timeout value is different than 2007 and we were using too low of a value for SP 2010 to return any results. Thus, that caused us to page through an infinite number of pages of size zero.
diff --git a/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java b/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java index 4cb1484..9471570 100644 --- a/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java +++ b/src/com/google/enterprise/adaptor/sharepoint/SharePointAdaptor.java
@@ -191,6 +191,11 @@ private final HttpClient httpClient; private boolean xmlValidation; private NtlmAuthenticator ntlmAuthenticator; + /** + * Cached value of whether we are talking to a SP 2010 server or not. This + * value is used in case of error in certain situations. + */ + private boolean isSp2010; public SharePointAdaptor() { this(new SiteDataFactoryImpl(), new HttpClientImpl()); @@ -297,9 +302,17 @@ Set<String> discoveredContentDatabases; if (vs == null) { // Retrieving list of databases failed, but we can continue without it. + // We don't set isSp2010 here, because we don't know what version of + // server we are talking to. However, if isSp2010 is still its default, + // then contentDatabaseChangeId is also its default and is empty. When + // contentDatabaseChangeId is empty, we won't end up using isSp2010. discoveredContentDatabases = new HashSet<String>(contentDatabaseChangeId.keySet()); } else { + String version = vs.getMetadata().getVersion(); + // Version is missing for SP 2007 (but its version is 12). SP 2010 is 14. + isSp2010 = version != null && version.startsWith("14."); + discoveredContentDatabases = new HashSet<String>(); if (vs.getContentDatabases() != null) { for (ContentDatabases.ContentDatabase cd @@ -350,7 +363,8 @@ continue; } CursorPaginator<SPContentDatabase, String> changesPaginator - = client.getChangesContentDatabase(contentDatabase, changeId); + = client.getChangesContentDatabase(contentDatabase, changeId, + isSp2010); SPContentDatabase changes; try { while ((changes = changesPaginator.next()) != null) { @@ -1379,7 +1393,7 @@ */ private CursorPaginator<SPContentDatabase, String> getChangesContentDatabase(final String contentDatabaseGuid, - String startChangeId) { + String startChangeId, final boolean isSp2010) { log.entering("SiteDataClient", "getChangesContentDatabase", new Object[] {contentDatabaseGuid, startChangeId}); final Holder<String> lastChangeId = new Holder<String>(startChangeId); @@ -1390,18 +1404,19 @@ return new CursorPaginator<SPContentDatabase, String>() { @Override public SPContentDatabase next() throws IOException { - // SharePoint 2010 sometimes does not set lastChangeId=currentChangeId - // nor moreChanges=false when paging is complete, even though both of - // these conditions are a "MUST" requirement in the documentation. - // Thus, we make sure that each call changes the lastChangeId. - if (!moreChanges.value - || lastChangeId.value.equals(lastLastChangeId.value)) { + if (!moreChanges.value) { return null; } lastLastChangeId.value = lastChangeId.value; Holder<String> result = new Holder<String>(); + // In non-SP2010, the timeout is a number of seconds. In SP2010, the + // timeout is n * 60, where n is the number of items you want + // returned. However, in SP2010, asking for more than 10 items seems + // to lose results. If timeout is less than 60 in SP 2010, then it + // causes an infinite loop. + int timeout = isSp2010 ? 10 * 60 : 15; siteData.getChanges(ObjectType.CONTENT_DATABASE, contentDatabaseGuid, - lastChangeId, currentChangeId, 15, result, moreChanges); + lastChangeId, currentChangeId, timeout, result, moreChanges); // XmlProcessingExceptions fine after this point. String xml = result.value; xml = xml.replace("<SPContentDatabase ",
diff --git a/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java b/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java index 1bb7ac9..83af49f 100644 --- a/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java +++ b/test/com/google/enterprise/adaptor/sharepoint/SharePointAdaptorTest.java
@@ -3361,11 +3361,13 @@ } @Test - public void testModifiedGetDocIdsBrokenSP2010() throws IOException, + public void testModifiedGetDocIdsSP2010() throws IOException, InterruptedException { final String getContentVirtualServer = "<VirtualServer>" - + "<Metadata URL=\"http://localhost:1/\" />" + + "<Metadata ID=\"{3a125232-0c27-495f-8c92-65ad85b5a17c}\"" + + " Version=\"14.0.4762.1000\" URL=\"http://localhost:1/\"" + + " URLZone=\"Default\" URLIsHostHeader=\"False\" />" + "<ContentDatabases>" + "<ContentDatabase ID=\"{4fb7dea1-2912-4927-9eda-1ea2f0977cf8}\" />" + "</ContentDatabases>" @@ -3420,20 +3422,20 @@ Integer timeout, Holder<String> getChangesResult, Holder<Boolean> moreChanges) { atomicNumberGetChangesCalls.getAndIncrement(); + // The timeout in SP 2010 is not a timeout and should always be at least + // 60. Otherwise, you will always get zero results. + assertTrue(timeout >= 60); assertEquals(ObjectType.CONTENT_DATABASE, objectType); assertEquals("{4fb7dea1-2912-4927-9eda-1ea2f0977cf8}", contentDatabaseId); assertEquals( "1;0;4fb7dea1-2912-4927-9eda-1ea2f0977cf8;634727056594000000;603", lastChangeId.value); - // Purposefully make lastChangeId != currentChangeId, because SP 2010 - // has been known to do this. setValue(currentChangeId, "1;0;4fb7dea1-2912-4927-9eda-1ea2f0977cf9;634" + "727056595000000;604"); + setValue(lastChangeId, currentChangeId.value); setValue(getChangesResult, getChangesContentDatabase4fb); - // Purposefully make moreChanges=true even though there are no more - // pages, because SP 2010 has been known to do this. - setValue(moreChanges, true); + setValue(moreChanges, false); } }; SiteDataFactory siteDataFactory = new SingleSiteDataFactory(siteData,