This is an automated email from the ASF dual-hosted git repository. juanpablo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/jspwiki.git
commit 718925b1c9951292b193662ebdea154a94d6ce71 Author: Juan Pablo Santos RodrÃguez <[email protected]> AuthorDate: Sat Apr 23 22:55:56 2022 +0200 JSPWIKI-1171: Ensure Lucene indexes all pages and attachments, even when they don't fit in the cache --- .../wiki/attachment/DefaultAttachmentManager.java | 2 +- .../wiki/providers/CachingAttachmentProvider.java | 55 +++-- .../org/apache/wiki/providers/CachingProvider.java | 76 +++---- .../apache/wiki/search/LuceneSearchProvider.java | 226 ++++++++++----------- .../apache/wiki/providers/CachingProviderTest.java | 44 +++- .../src/test/resources/ehcache-jspwiki-small.xml | 71 +++++++ 6 files changed, 289 insertions(+), 185 deletions(-) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/attachment/DefaultAttachmentManager.java b/jspwiki-main/src/main/java/org/apache/wiki/attachment/DefaultAttachmentManager.java index e0ccfb658..6fb3dd935 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/attachment/DefaultAttachmentManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/attachment/DefaultAttachmentManager.java @@ -282,7 +282,7 @@ public class DefaultAttachmentManager implements AttachmentManager { /** {@inheritDoc} */ @Override - public Collection<Attachment> getAllAttachments() throws ProviderException { + public Collection< Attachment > getAllAttachments() throws ProviderException { if( attachmentsEnabled() ) { return m_provider.listAllChanged( new Date( 0L ) ); } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java index 51fc64466..9d1d264de 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingAttachmentProvider.java @@ -26,6 +26,7 @@ import org.apache.wiki.api.core.Page; import org.apache.wiki.api.exceptions.NoRequiredPropertyException; import org.apache.wiki.api.exceptions.ProviderException; import org.apache.wiki.api.providers.AttachmentProvider; +import org.apache.wiki.api.providers.PageProvider; import org.apache.wiki.api.providers.WikiProvider; import org.apache.wiki.api.search.QueryItem; import org.apache.wiki.attachment.AttachmentManager; @@ -43,6 +44,7 @@ import java.util.Date; import java.util.List; import java.util.NoSuchElementException; import java.util.Properties; +import java.util.concurrent.atomic.AtomicLong; /** @@ -55,9 +57,10 @@ public class CachingAttachmentProvider implements AttachmentProvider { private static final Logger LOG = LogManager.getLogger( CachingAttachmentProvider.class ); - private AttachmentProvider m_provider; + private AttachmentProvider provider; private CachingManager cachingManager; - private boolean m_gotall; + private boolean allRequested; + private final AtomicLong attachments = new AtomicLong( 0L ); /** * {@inheritDoc} @@ -76,9 +79,9 @@ public class CachingAttachmentProvider implements AttachmentProvider { } try { - m_provider = ClassUtil.buildInstance( "org.apache.wiki.providers", classname ); - LOG.debug( "Initializing real provider class {}", m_provider ); - m_provider.initialize( engine, properties ); + provider = ClassUtil.buildInstance( "org.apache.wiki.providers", classname ); + LOG.debug( "Initializing real provider class {}", provider ); + provider.initialize( engine, properties ); } catch( final ReflectiveOperationException e ) { LOG.error( "Unable to instantiate provider class {}", classname, e ); throw new IllegalArgumentException( "illegal provider class", e ); @@ -90,10 +93,11 @@ public class CachingAttachmentProvider implements AttachmentProvider { */ @Override public void putAttachmentData( final Attachment att, final InputStream data ) throws ProviderException, IOException { - m_provider.putAttachmentData( att, data ); + provider.putAttachmentData( att, data ); cachingManager.remove( CachingManager.CACHE_ATTACHMENTS_COLLECTION, att.getParentName() ); att.setLastModified( new Date() ); cachingManager.put( CachingManager.CACHE_ATTACHMENTS, att.getName(), att ); + attachments.incrementAndGet(); } /** @@ -101,7 +105,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { */ @Override public InputStream getAttachmentData( final Attachment att ) throws ProviderException, IOException { - return m_provider.getAttachmentData( att ); + return provider.getAttachmentData( att ); } /** @@ -111,7 +115,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { public List< Attachment > listAttachments( final Page page ) throws ProviderException { LOG.debug( "Listing attachments for {}", page ); final List< Attachment > atts = cachingManager.get( CachingManager.CACHE_ATTACHMENTS_COLLECTION, page.getName(), - () -> m_provider.listAttachments( page ) ); + () -> provider.listAttachments( page ) ); return cloneCollection( atts ); } @@ -124,7 +128,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { */ @Override public Collection< Attachment > findAttachments( final QueryItem[] query ) { - return m_provider.findAttachments( query ); + return provider.findAttachments( query ); } /** @@ -133,15 +137,18 @@ public class CachingAttachmentProvider implements AttachmentProvider { @Override public List< Attachment > listAllChanged( final Date timestamp ) throws ProviderException { final List< Attachment > all; - if ( !m_gotall ) { - all = m_provider.listAllChanged( timestamp ); + if ( !allRequested ) { + all = provider.listAllChanged( timestamp ); // Make sure that all attachments are in the cache. synchronized( this ) { for( final Attachment att : all ) { cachingManager.put( CachingManager.CACHE_ATTACHMENTS, att.getName(), att ); } - m_gotall = true; + if( timestamp.getTime() == 0L ) { // all attachments requested + allRequested = true; + attachments.set( all.size() ); + } } } else { final List< String > keys = cachingManager.keys( CachingManager.CACHE_ATTACHMENTS ); @@ -155,11 +162,11 @@ public class CachingAttachmentProvider implements AttachmentProvider { } if( cachingManager.enabled( CachingManager.CACHE_ATTACHMENTS ) - && all.size() >= cachingManager.info( CachingManager.CACHE_ATTACHMENTS ).getMaxElementsAllowed() ) { + && attachments.get() >= cachingManager.info( CachingManager.CACHE_ATTACHMENTS ).getMaxElementsAllowed() ) { LOG.warn( "seems {} can't hold all attachments from your page repository, " + "so we're delegating on the underlying provider instead. Please consider increasing " + "your cache sizes on the ehcache configuration file to avoid this behaviour", CachingManager.CACHE_ATTACHMENTS ); - return m_provider.listAllChanged( timestamp ); + return provider.listAllChanged( timestamp ); } return all; @@ -192,10 +199,10 @@ public class CachingAttachmentProvider implements AttachmentProvider { // We don't cache previous versions if( version != WikiProvider.LATEST_VERSION ) { LOG.debug( "...we don't cache old versions" ); - return m_provider.getAttachmentInfo( page, name, version ); + return provider.getAttachmentInfo( page, name, version ); } final Collection< Attachment > c = cachingManager.get( CachingManager.CACHE_ATTACHMENTS_COLLECTION, page.getName(), - ()-> m_provider.listAttachments( page ) ); + ()-> provider.listAttachments( page ) ); return findAttachmentFromCollection( c, name ); } @@ -204,7 +211,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { */ @Override public List< Attachment > getVersionHistory( final Attachment att ) { - return m_provider.getVersionHistory( att ); + return provider.getVersionHistory( att ); } /** @@ -214,7 +221,10 @@ public class CachingAttachmentProvider implements AttachmentProvider { public void deleteVersion( final Attachment att ) throws ProviderException { // This isn't strictly speaking correct, but it does not really matter cachingManager.remove( CachingManager.CACHE_ATTACHMENTS_COLLECTION, att.getParentName() ); - m_provider.deleteVersion( att ); + provider.deleteVersion( att ); + if( att.getVersion() == PageProvider.LATEST_VERSION ) { + attachments.decrementAndGet(); + } } /** @@ -224,7 +234,8 @@ public class CachingAttachmentProvider implements AttachmentProvider { public void deleteAttachment( final Attachment att ) throws ProviderException { cachingManager.remove( CachingManager.CACHE_ATTACHMENTS_COLLECTION, att.getParentName() ); cachingManager.remove( CachingManager.CACHE_ATTACHMENTS, att.getName() ); - m_provider.deleteAttachment( att ); + provider.deleteAttachment( att ); + attachments.decrementAndGet(); } /** @@ -236,7 +247,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { public synchronized String getProviderInfo() { final CacheInfo attCacheInfo = cachingManager.info( CachingManager.CACHE_ATTACHMENTS ); final CacheInfo attColCacheInfo = cachingManager.info( CachingManager.CACHE_ATTACHMENTS_COLLECTION ); - return "Real provider: " + m_provider.getClass().getName() + + return "Real provider: " + provider.getClass().getName() + ". Attachment cache hits: " + attCacheInfo.getHits() + ". Attachment cache misses: " + attCacheInfo.getMisses() + ". Attachment collection cache hits: " + attColCacheInfo.getHits() + @@ -249,7 +260,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { * @return The real provider underneath this one. */ public AttachmentProvider getRealProvider() { - return m_provider; + return provider; } /** @@ -257,7 +268,7 @@ public class CachingAttachmentProvider implements AttachmentProvider { */ @Override public void moveAttachmentsForPage( final String oldParent, final String newParent ) throws ProviderException { - m_provider.moveAttachmentsForPage( oldParent, newParent ); + provider.moveAttachmentsForPage( oldParent, newParent ); cachingManager.remove( CachingManager.CACHE_ATTACHMENTS_COLLECTION, newParent ); cachingManager.remove( CachingManager.CACHE_ATTACHMENTS_COLLECTION, oldParent ); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java index 1be3b299a..5b1260f46 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/providers/CachingProvider.java @@ -44,6 +44,7 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Properties; import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicLong; /** @@ -63,11 +64,11 @@ public class CachingProvider implements PageProvider { private static final Logger LOG = LogManager.getLogger( CachingProvider.class ); private CachingManager cachingManager; - private PageProvider m_provider; - private Engine m_engine; + private PageProvider provider; + private Engine engine; - // FIXME: This MUST be cached somehow. - private boolean m_gotall; + private boolean allRequested; + private final AtomicLong pages = new AtomicLong( 0L ); /** * {@inheritDoc} @@ -77,8 +78,8 @@ public class CachingProvider implements PageProvider { LOG.debug( "Initing CachingProvider" ); // engine is used for getting the search engine - m_engine = engine; - cachingManager = m_engine.getManager( CachingManager.class ); + this.engine = engine; + cachingManager = this.engine.getManager( CachingManager.class ); // Find and initialize real provider. final String classname; @@ -89,9 +90,9 @@ public class CachingProvider implements PageProvider { } try { - m_provider = ClassUtil.buildInstance( "org.apache.wiki.providers", classname ); - LOG.debug( "Initializing real provider class {}", m_provider ); - m_provider.initialize( engine, properties ); + provider = ClassUtil.buildInstance( "org.apache.wiki.providers", classname ); + LOG.debug( "Initializing real provider class {}", provider ); + provider.initialize( engine, properties ); } catch( final ReflectiveOperationException e ) { LOG.error( "Unable to instantiate provider class {}", classname, e ); throw new IllegalArgumentException( "illegal provider class", e ); @@ -103,7 +104,7 @@ public class CachingProvider implements PageProvider { if( name == null ) { return null; } - return cachingManager.get( CachingManager.CACHE_PAGES, name, () -> m_provider.getPageInfo( name, PageProvider.LATEST_VERSION ) ); + return cachingManager.get( CachingManager.CACHE_PAGES, name, () -> provider.getPageInfo( name, PageProvider.LATEST_VERSION ) ); } @@ -130,12 +131,13 @@ public class CachingProvider implements PageProvider { return true; } - return m_provider.pageExists( pageName, version ); + return provider.pageExists( pageName, version ); } try { return getPageInfo( pageName, version ) != null; } catch( final ProviderException e ) { + LOG.info( "Provider failed while retrieving {}", pageName ); } return false; @@ -164,14 +166,14 @@ public class CachingProvider implements PageProvider { } // If we have a list of all pages in memory, then any page not in the cache must be non-existent. - if( m_gotall ) { + if( pages.get() < cachingManager.info( CachingManager.CACHE_PAGES ).getMaxElementsAllowed() ) { return false; } // We could add the page to the cache here as well, but in order to understand whether that is a good thing or not we would // need to analyze the JSPWiki calling patterns extensively. Presumably it would be a good thing if pageExists() is called // many times before the first getPageText() is called, and the whole page is cached. - return m_provider.pageExists( pageName ); + return provider.pageExists( pageName ); } /** @@ -193,7 +195,7 @@ public class CachingProvider implements PageProvider { if( p != null && p.getVersion() == version ) { result = getTextFromCache( pageName ); } else { - result = m_provider.getPageText( pageName, version ); + result = provider.getPageText( pageName, version ); } } @@ -207,7 +209,7 @@ public class CachingProvider implements PageProvider { return cachingManager.get( CachingManager.CACHE_PAGES_TEXT, pageName, () -> { if( pageExists( pageName ) ) { - return m_provider.getPageText( pageName, PageProvider.LATEST_VERSION ); + return provider.getPageText( pageName, PageProvider.LATEST_VERSION ); } return null; } ); @@ -219,7 +221,7 @@ public class CachingProvider implements PageProvider { @Override public void putPageText( final Page page, final String text ) throws ProviderException { synchronized( this ) { - m_provider.putPageText( page, text ); + provider.putPageText( page, text ); page.setLastModified( new Date() ); // Refresh caches properly @@ -229,6 +231,7 @@ public class CachingProvider implements PageProvider { getPageInfoFromCache( page.getName() ); } + pages.incrementAndGet(); } /** @@ -237,15 +240,16 @@ public class CachingProvider implements PageProvider { @Override public Collection< Page > getAllPages() throws ProviderException { final Collection< Page > all; - if ( !m_gotall ) { - all = m_provider.getAllPages(); + if ( !allRequested ) { + all = provider.getAllPages(); // Make sure that all pages are in the cache. synchronized( this ) { for( final Page p : all ) { cachingManager.put( CachingManager.CACHE_PAGES, p.getName(), p ); } - m_gotall = true; + allRequested = true; } + pages.set( all.size() ); } else { final List< String > keys = cachingManager.keys( CachingManager.CACHE_PAGES ); all = new TreeSet<>(); @@ -258,11 +262,11 @@ public class CachingProvider implements PageProvider { } if( cachingManager.enabled( CachingManager.CACHE_PAGES ) - && all.size() >= cachingManager.info( CachingManager.CACHE_PAGES ).getMaxElementsAllowed() ) { + && pages.get() >= cachingManager.info( CachingManager.CACHE_PAGES ).getMaxElementsAllowed() ) { LOG.warn( "seems {} can't hold all pages from your page repository, " + "so we're delegating on the underlying provider instead. Please consider increasing " + "your cache sizes on the ehcache configuration file to avoid this behaviour", CachingManager.CACHE_PAGES ); - return m_provider.getAllPages(); + return provider.getAllPages(); } return all; @@ -273,7 +277,7 @@ public class CachingProvider implements PageProvider { */ @Override public Collection< Page > getAllChangedSince( final Date date ) { - return m_provider.getAllChangedSince( date ); + return provider.getAllChangedSince( date ); } /** @@ -281,7 +285,7 @@ public class CachingProvider implements PageProvider { */ @Override public int getPageCount() throws ProviderException { - return m_provider.getPageCount(); + return provider.getPageCount(); } /** @@ -290,17 +294,17 @@ public class CachingProvider implements PageProvider { @Override public Collection< SearchResult > findPages( final QueryItem[] query ) { // If the provider is a fast searcher, then just pass this request through. - return m_provider.findPages( query ); + return provider.findPages( query ); // FIXME: Does not implement fast searching } // FIXME: Kludge: make sure that the page is also parsed and it gets all the necessary variables. private void refreshMetadata( final Page page ) { if( page != null && !page.hasMetadata() ) { - final RenderingManager mgr = m_engine.getManager( RenderingManager.class ); + final RenderingManager mgr = engine.getManager( RenderingManager.class ); try { - final String data = m_provider.getPageText( page.getName(), page.getVersion() ); - final Context ctx = Wiki.context().create( m_engine, page ); + final String data = provider.getPageText( page.getName(), page.getVersion() ); + final Context ctx = Wiki.context().create( engine, page ); final MarkupParser parser = mgr.getParser( ctx, data ); parser.parse(); @@ -322,7 +326,7 @@ public class CachingProvider implements PageProvider { page = cached; } else { // We do not cache old versions. - page = m_provider.getPageInfo( pageName, version ); + page = provider.getPageInfo( pageName, version ); } refreshMetadata( page ); return page; @@ -336,7 +340,7 @@ public class CachingProvider implements PageProvider { if( pageName == null ) { return null; } - return cachingManager.get( CachingManager.CACHE_PAGES_HISTORY, pageName, () -> m_provider.getVersionHistory( pageName ) ); + return cachingManager.get( CachingManager.CACHE_PAGES_HISTORY, pageName, () -> provider.getVersionHistory( pageName ) ); } /** @@ -348,7 +352,7 @@ public class CachingProvider implements PageProvider { public synchronized String getProviderInfo() { final CacheInfo pageCacheInfo = cachingManager.info( CachingManager.CACHE_PAGES ); final CacheInfo pageHistoryCacheInfo = cachingManager.info( CachingManager.CACHE_PAGES_HISTORY ); - return "Real provider: " + m_provider.getClass().getName()+ + return "Real provider: " + provider.getClass().getName()+ ". Page cache hits: " + pageCacheInfo.getHits() + ". Page cache misses: " + pageCacheInfo.getMisses() + ". History cache hits: " + pageHistoryCacheInfo.getHits() + @@ -371,9 +375,12 @@ public class CachingProvider implements PageProvider { cachingManager.remove( CachingManager.CACHE_PAGES_TEXT, pageName ); } - m_provider.deleteVersion( pageName, version ); + provider.deleteVersion( pageName, version ); cachingManager.remove( CachingManager.CACHE_PAGES_HISTORY, pageName ); } + if( version == PageProvider.LATEST_VERSION ) { + pages.decrementAndGet(); + } } /** @@ -386,8 +393,9 @@ public class CachingProvider implements PageProvider { cachingManager.put( CachingManager.CACHE_PAGES, pageName, null ); cachingManager.put( CachingManager.CACHE_PAGES_TEXT, pageName, null ); cachingManager.put( CachingManager.CACHE_PAGES_HISTORY, pageName, null ); - m_provider.deletePage( pageName ); + provider.deletePage( pageName ); } + pages.decrementAndGet(); } /** @@ -395,7 +403,7 @@ public class CachingProvider implements PageProvider { */ @Override public void movePage( final String from, final String to ) throws ProviderException { - m_provider.movePage( from, to ); + provider.movePage( from, to ); synchronized( this ) { // Clear any cached version of the old page and new page cachingManager.remove( CachingManager.CACHE_PAGES, from ); @@ -415,7 +423,7 @@ public class CachingProvider implements PageProvider { * @return The real provider. */ public PageProvider getRealProvider() { - return m_provider; + return provider; } } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java b/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java index dbf6ec705..6183d13f0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/search/LuceneSearchProvider.java @@ -85,13 +85,13 @@ import java.util.concurrent.Executors; /** - * Interface for the search providers that handle searching the Wiki + * Interface for the search providers that handle searching the Wiki * - * @since 2.2.21. + * @since 2.2.21. */ public class LuceneSearchProvider implements SearchProvider { - protected static final Logger log = LogManager.getLogger(LuceneSearchProvider.class); + protected static final Logger log = LogManager.getLogger( LuceneSearchProvider.class ); private Engine m_engine; private Executor searchExecutor; @@ -127,18 +127,16 @@ public class LuceneSearchProvider implements SearchProvider { /** The maximum number of hits to return from searches. */ public static final int MAX_SEARCH_HITS = 99_999; - - private static final String PUNCTUATION_TO_SPACES = StringUtils.repeat(" ", TextUtil.PUNCTUATION_CHARS_ALLOWED.length() ); - /** - * {@inheritDoc} - */ + private static final String PUNCTUATION_TO_SPACES = StringUtils.repeat( " ", TextUtil.PUNCTUATION_CHARS_ALLOWED.length() ); + + /** {@inheritDoc} */ @Override - public void initialize( final Engine engine, final Properties props ) throws NoRequiredPropertyException, IOException { + public void initialize( final Engine engine, final Properties props ) throws NoRequiredPropertyException, IOException { m_engine = engine; searchExecutor = Executors.newCachedThreadPool(); - m_luceneDirectory = engine.getWorkDir()+File.separator+LUCENE_DIR; + m_luceneDirectory = engine.getWorkDir() + File.separator + LUCENE_DIR; final int initialDelay = TextUtil.getIntegerProperty( props, PROP_LUCENE_INITIALDELAY, LuceneUpdater.INITIAL_DELAY ); final int indexDelay = TextUtil.getIntegerProperty( props, PROP_LUCENE_INDEXDELAY, LuceneUpdater.INDEX_DELAY ); @@ -146,53 +144,51 @@ public class LuceneSearchProvider implements SearchProvider { m_analyzerClass = TextUtil.getStringProperty( props, PROP_LUCENE_ANALYZER, m_analyzerClass ); // FIXME: Just to be simple for now, we will do full reindex only if no files are in lucene directory. - final File dir = new File(m_luceneDirectory); - log.info("Lucene enabled, cache will be in: "+dir.getAbsolutePath()); + final File dir = new File( m_luceneDirectory ); + log.info( "Lucene enabled, cache will be in: {}", dir.getAbsolutePath() ); try { if( !dir.exists() ) { dir.mkdirs(); } if( !dir.exists() || !dir.canWrite() || !dir.canRead() ) { - log.error("Cannot write to Lucene directory, disabling Lucene: "+dir.getAbsolutePath()); + log.error( "Cannot write to Lucene directory, disabling Lucene: {}", dir.getAbsolutePath() ); throw new IOException( "Invalid Lucene directory." ); } final String[] filelist = dir.list(); if( filelist == null ) { - throw new IOException( "Invalid Lucene directory: cannot produce listing: "+dir.getAbsolutePath()); + throw new IOException( "Invalid Lucene directory: cannot produce listing: " + dir.getAbsolutePath() ); } } catch( final IOException e ) { - log.error("Problem while creating Lucene index - not using Lucene.", e); + log.error( "Problem while creating Lucene index - not using Lucene.", e ); } - // Start the Lucene update thread, which waits first - // for a little while before starting to go through + // Start the Lucene update thread, which waits first for a little while before starting to go through // the Lucene "pages that need updating". final LuceneUpdater updater = new LuceneUpdater( m_engine, this, initialDelay, indexDelay ); updater.start(); } /** - * Returns the handling engine. + * Returns the handling engine. * - * @return Current Engine + * @return Current Engine */ - protected Engine getEngine() - { + protected Engine getEngine() { return m_engine; } /** - * Performs a full Lucene reindex, if necessary. + * Performs a full Lucene reindex, if necessary. * - * @throws IOException If there's a problem during indexing + * @throws IOException If there's a problem during indexing */ protected void doFullLuceneReindex() throws IOException { - final File dir = new File(m_luceneDirectory); + final File dir = new File( m_luceneDirectory ); final String[] filelist = dir.list(); if( filelist == null ) { - throw new IOException( "Invalid Lucene directory: cannot produce listing: "+dir.getAbsolutePath()); + throw new IOException( "Invalid Lucene directory: cannot produce listing: " + dir.getAbsolutePath() ); } try { @@ -202,56 +198,60 @@ public class LuceneSearchProvider implements SearchProvider { // final Date start = new Date(); - log.info("Starting Lucene reindexing, this can take a couple of minutes..."); + log.info( "Starting Lucene reindexing, this can take a couple of minutes..." ); final Directory luceneDir = new NIOFSDirectory( dir.toPath() ); try( final IndexWriter writer = getIndexWriter( luceneDir ) ) { + long pagesIndexed = 0L; final Collection< Page > allPages = m_engine.getManager( PageManager.class ).getAllPages(); for( final Page page : allPages ) { try { final String text = m_engine.getManager( PageManager.class ).getPageText( page.getName(), WikiProvider.LATEST_VERSION ); luceneIndexPage( page, text, writer ); + pagesIndexed++; } catch( final IOException e ) { - log.warn( "Unable to index page " + page.getName() + ", continuing to next ", e ); + log.warn( "Unable to index page {}, continuing to next ", page.getName(), e ); } } + log.info( "Indexed {} pages", pagesIndexed ); + long attachmentsIndexed = 0L; final Collection< Attachment > allAttachments = m_engine.getManager( AttachmentManager.class ).getAllAttachments(); for( final Attachment att : allAttachments ) { try { final String text = getAttachmentContent( att.getName(), WikiProvider.LATEST_VERSION ); luceneIndexPage( att, text, writer ); + attachmentsIndexed++; } catch( final IOException e ) { - log.warn( "Unable to index attachment " + att.getName() + ", continuing to next", e ); + log.warn( "Unable to index attachment {}, continuing to next", att.getName(), e ); } } - + log.info( "Indexed {} attachments", attachmentsIndexed ); } final Date end = new Date(); - log.info( "Full Lucene index finished in " + (end.getTime() - start.getTime()) + " milliseconds." ); + log.info( "Full Lucene index finished in {} milliseconds.", end.getTime() - start.getTime() ); } else { - log.info("Files found in Lucene directory, not reindexing."); + log.info( "Files found in Lucene directory, not reindexing." ); } - } catch ( final IOException e ) { - log.error("Problem while creating Lucene index - not using Lucene.", e); - } catch ( final ProviderException e ) { - log.error("Problem reading pages while creating Lucene index (JSPWiki won't start.)", e); - throw new IllegalArgumentException("unable to create Lucene index"); + } catch( final IOException e ) { + log.error( "Problem while creating Lucene index - not using Lucene.", e ); + } catch( final ProviderException e ) { + log.error( "Problem reading pages while creating Lucene index (JSPWiki won't start.)", e ); + throw new IllegalArgumentException( "unable to create Lucene index" ); } catch( final Exception e ) { - log.error("Unable to start lucene",e); + log.error( "Unable to start lucene", e ); } } /** - * Fetches the attachment content from the repository. - * Content is flat text that can be used for indexing/searching or display - * - * @param attachmentName Name of the attachment. - * @param version The version of the attachment. - * - * @return the content of the Attachment as a String. + * Fetches the attachment content from the repository. + * Content is flat text that can be used for indexing/searching or display + * + * @param attachmentName Name of the attachment. + * @param version The version of the attachment. + * @return the content of the Attachment as a String. */ protected String getAttachmentContent( final String attachmentName, final int version ) { final AttachmentManager mgr = m_engine.getManager( AttachmentManager.class ); @@ -262,7 +262,7 @@ public class LuceneSearchProvider implements SearchProvider { return getAttachmentContent( att ); } } catch( final ProviderException e ) { - log.error("Attachment cannot be loaded", e); + log.error( "Attachment cannot be loaded", e ); } return null; } @@ -293,7 +293,7 @@ public class LuceneSearchProvider implements SearchProvider { FileUtil.copyContents( new InputStreamReader( attStream ), sout ); out = out + " " + sout; } catch( final ProviderException | IOException e ) { - log.error("Attachment cannot be loaded", e); + log.error( "Attachment cannot be loaded", e ); } } @@ -301,13 +301,13 @@ public class LuceneSearchProvider implements SearchProvider { } /** - * Updates the lucene index for a single page. + * Updates the lucene index for a single page. * - * @param page The WikiPage to check - * @param text The page text to index. + * @param page The WikiPage to check + * @param text The page text to index. */ protected synchronized void updateLuceneIndex( final Page page, final String text ) { - log.debug("Updating Lucene index for page '" + page.getName() + "'..."); + log.debug( "Updating Lucene index for page '{}'...", page.getName() ); pageRemoved( page ); // Now add back the new version. @@ -315,14 +315,14 @@ public class LuceneSearchProvider implements SearchProvider { final IndexWriter writer = getIndexWriter( luceneDir ) ) { luceneIndexPage( page, text, writer ); } catch( final IOException e ) { - log.error("Unable to update page '" + page.getName() + "' from Lucene index", e); + log.error( "Unable to update page '{}' from Lucene index", page.getName(), e ); // reindexPage( page ); } catch( final Exception e ) { - log.error("Unexpected Lucene exception - please check configuration!",e); + log.error( "Unexpected Lucene exception - please check configuration!", e ); // reindexPage( page ); } - log.debug("Done updating Lucene index for page '" + page.getName() + "'."); + log.debug( "Done updating Lucene index for page '{}'.", page.getName() ); } private Analyzer getLuceneAnalyzer() throws ProviderException { @@ -336,13 +336,13 @@ public class LuceneSearchProvider implements SearchProvider { } /** - * Indexes page using the given IndexWriter. + * Indexes page using the given IndexWriter. * - * @param page WikiPage - * @param text Page text to index - * @param writer The Lucene IndexWriter to use for indexing - * @return the created index Document - * @throws IOException If there's an indexing problem + * @param page WikiPage + * @param text Page text to index + * @param writer The Lucene IndexWriter to use for indexing + * @return the created index Document + * @throws IOException If there's an indexing problem */ protected Document luceneIndexPage( final Page page, final String text, final IndexWriter writer ) throws IOException { log.debug( "Indexing {}...", page.getName() ); @@ -396,14 +396,14 @@ public class LuceneSearchProvider implements SearchProvider { doc.add( field ); } synchronized( writer ) { - writer.addDocument(doc); + writer.addDocument( doc ); } return doc; } /** - * {@inheritDoc} + * {@inheritDoc} */ @Override public synchronized void pageRemoved( final Page page ) { @@ -411,21 +411,21 @@ public class LuceneSearchProvider implements SearchProvider { final IndexWriter writer = getIndexWriter( luceneDir ) ) { final Query query = new TermQuery( new Term( LUCENE_ID, page.getName() ) ); writer.deleteDocuments( query ); - } catch ( final Exception e ) { - log.error("Unable to remove page '" + page.getName() + "' from Lucene index", e); + } catch( final Exception e ) { + log.error( "Unable to remove page '{}' from Lucene index", page.getName(), e ); } } - - IndexWriter getIndexWriter(final Directory luceneDir ) throws IOException, ProviderException { + + IndexWriter getIndexWriter( final Directory luceneDir ) throws IOException, ProviderException { final IndexWriterConfig writerConfig = new IndexWriterConfig( getLuceneAnalyzer() ); writerConfig.setOpenMode( OpenMode.CREATE_OR_APPEND ); return new IndexWriter( luceneDir, writerConfig ); } - + /** - * Adds a page-text pair to the lucene update queue. Safe to call always + * Adds a page-text pair to the lucene update queue. Safe to call always * - * @param page WikiPage to add to the update queue. + * @param page WikiPage to add to the update queue. */ @Override public void reindexPage( final Page page ) { @@ -434,46 +434,41 @@ public class LuceneSearchProvider implements SearchProvider { // TODO: Think if this was better done in the thread itself? if( page instanceof Attachment ) { - text = getAttachmentContent( ( Attachment )page ); + text = getAttachmentContent( ( Attachment ) page ); } else { text = m_engine.getManager( PageManager.class ).getPureText( page ); } if( text != null ) { // Add work item to m_updates queue. - final Object[] pair = new Object[2]; - pair[0] = page; - pair[1] = text; + final Object[] pair = new Object[ 2 ]; + pair[ 0 ] = page; + pair[ 1 ] = text; m_updates.add( pair ); - log.debug("Scheduling page " + page.getName() + " for index update"); + log.debug( "Scheduling page {} for index update", page.getName() ); } } } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override public Collection< SearchResult > findPages( final String query, final Context wikiContext ) throws ProviderException { return findPages( query, FLAG_CONTEXTS, wikiContext ); } - /** - * Create contexts also. Generating contexts can be expensive, - * so they're not on by default. - */ + /** Create contexts also. Generating contexts can be expensive, so they're not on by default. */ public static final int FLAG_CONTEXTS = 0x01; /** - * Searches pages using a particular combination of flags. + * Searches pages using a particular combination of flags. * - * @param query The query to perform in Lucene query language - * @param flags A set of flags - * @return A Collection of SearchResult instances - * @throws ProviderException if there is a problem with the backend + * @param query The query to perform in Lucene query language + * @param flags A set of flags + * @return A Collection of SearchResult instances + * @throws ProviderException if there is a problem with the backend */ public Collection< SearchResult > findPages( final String query, final int flags, final Context wikiContext ) throws ProviderException { - ArrayList<SearchResult> list = null; + ArrayList< SearchResult > list = null; Highlighter highlighter = null; try( final Directory luceneDir = new NIOFSDirectory( new File( m_luceneDirectory ).toPath() ); @@ -483,16 +478,16 @@ public class LuceneSearchProvider implements SearchProvider { final Query luceneQuery = qp.parse( query ); final IndexSearcher searcher = new IndexSearcher( reader, searchExecutor ); - if( (flags & FLAG_CONTEXTS) != 0 ) { - highlighter = new Highlighter(new SimpleHTMLFormatter("<span class=\"searchmatch\">", "</span>"), - new SimpleHTMLEncoder(), - new QueryScorer( luceneQuery ) ); + if( ( flags & FLAG_CONTEXTS ) != 0 ) { + highlighter = new Highlighter( new SimpleHTMLFormatter( "<span class=\"searchmatch\">", "</span>" ), + new SimpleHTMLEncoder(), + new QueryScorer( luceneQuery ) ); } - final ScoreDoc[] hits = searcher.search(luceneQuery, MAX_SEARCH_HITS).scoreDocs; + final ScoreDoc[] hits = searcher.search( luceneQuery, MAX_SEARCH_HITS ).scoreDocs; final AuthorizationManager mgr = m_engine.getManager( AuthorizationManager.class ); - list = new ArrayList<>(hits.length); + list = new ArrayList<>( hits.length ); for( final ScoreDoc hit : hits ) { final int docID = hit.doc; final Document doc = searcher.doc( docID ); @@ -502,15 +497,14 @@ public class LuceneSearchProvider implements SearchProvider { if( page != null ) { final PagePermission pp = new PagePermission( page, PagePermission.VIEW_ACTION ); if( mgr.checkPermission( wikiContext.getWikiSession(), pp ) ) { - final int score = ( int )( hit.score * 100 ); + final int score = ( int ) ( hit.score * 100 ); // Get highlighted search contexts final String text = doc.get( LUCENE_PAGE_CONTENTS ); String[] fragments = new String[ 0 ]; if( text != null && highlighter != null ) { - final TokenStream tokenStream = getLuceneAnalyzer() - .tokenStream( LUCENE_PAGE_CONTENTS, new StringReader( text ) ); + final TokenStream tokenStream = getLuceneAnalyzer().tokenStream( LUCENE_PAGE_CONTENTS, new StringReader( text ) ); fragments = highlighter.getBestFragments( tokenStream, text, MAX_FRAGMENTS ); } @@ -518,28 +512,25 @@ public class LuceneSearchProvider implements SearchProvider { list.add( result ); } } else { - log.error( "Lucene found a result page '" + pageName + "' that could not be loaded, removing from Lucene cache" ); + log.error( "Lucene found a result page '{}' that could not be loaded, removing from Lucene cache", pageName ); pageRemoved( Wiki.contents().page( m_engine, pageName ) ); } } } catch( final IOException e ) { - log.error("Failed during lucene search",e); + log.error( "Failed during lucene search", e ); } catch( final ParseException e ) { - log.info("Broken query; cannot parse query: " + query, e); + log.error( "Broken query; cannot parse query: {}", query, e ); throw new ProviderException( "You have entered a query Lucene cannot process [" + query + "]: " + e.getMessage() ); } catch( final InvalidTokenOffsetsException e ) { - log.error("Tokens are incompatible with provided text ",e); + log.error( "Tokens are incompatible with provided text ", e ); } return list; } - /** - * {@inheritDoc} - */ + /** {@inheritDoc} */ @Override - public String getProviderInfo() - { + public String getProviderInfo() { return "LuceneSearchProvider"; } @@ -559,7 +550,7 @@ public class LuceneSearchProvider implements SearchProvider { super( engine, indexDelay ); m_provider = provider; m_initialDelay = initialDelay; - setName("JSPWiki Lucene Indexer"); + setName( "JSPWiki Lucene Indexer" ); } @Override @@ -570,7 +561,7 @@ public class LuceneSearchProvider implements SearchProvider { try { Thread.sleep( m_initialDelay * 1000L ); } catch( final InterruptedException e ) { - throw new InternalWikiException("Interrupted while waiting to start.", e); + throw new InternalWikiException( "Interrupted while waiting to start.", e ); } m_watchdog.enterState( "Full reindex" ); @@ -581,14 +572,14 @@ public class LuceneSearchProvider implements SearchProvider { @Override public void backgroundTask() { - m_watchdog.enterState("Emptying index queue", 60); + m_watchdog.enterState( "Emptying index queue", 60 ); - synchronized ( m_provider.m_updates ) { + synchronized( m_provider.m_updates ) { while( m_provider.m_updates.size() > 0 ) { - final Object[] pair = m_provider.m_updates.remove(0); - final Page page = ( Page ) pair[0]; - final String text = ( String ) pair[1]; - m_provider.updateLuceneIndex(page, text); + final Object[] pair = m_provider.m_updates.remove( 0 ); + final Page page = ( Page )pair[ 0 ]; + final String text = ( String )pair[ 1 ]; + m_provider.updateLuceneIndex( page, text ); } } @@ -601,7 +592,7 @@ public class LuceneSearchProvider implements SearchProvider { private static class SearchResultImpl implements SearchResult { private final Page m_page; - private final int m_score; + private final int m_score; private final String[] m_contexts; public SearchResultImpl( final Page page, final int score, final String[] contexts ) { @@ -611,8 +602,7 @@ public class LuceneSearchProvider implements SearchProvider { } @Override - public Page getPage() - { + public Page getPage() { return m_page; } @@ -620,15 +610,13 @@ public class LuceneSearchProvider implements SearchProvider { * @see org.apache.wiki.SearchResult#getScore() */ @Override - public int getScore() - { + public int getScore() { return m_score; } @Override - public String[] getContexts() - { + public String[] getContexts() { return m_contexts; } } diff --git a/jspwiki-main/src/test/java/org/apache/wiki/providers/CachingProviderTest.java b/jspwiki-main/src/test/java/org/apache/wiki/providers/CachingProviderTest.java index d8ec6c550..f5d367c85 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/providers/CachingProviderTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/providers/CachingProviderTest.java @@ -35,13 +35,11 @@ import java.io.PrintWriter; import java.io.StringReader; import java.util.Properties; -public class CachingProviderTest { - - protected TestEngine testEngine = TestEngine.build(); +class CachingProviderTest { @AfterEach - public void tearDown() { - testEngine.deleteTestPage("Testi"); + void tearDown() { + TestEngine.emptyWikiDir(); TestEngine.emptyWorkDir(); } @@ -49,13 +47,13 @@ public class CachingProviderTest { * Checks that at startup we call the provider once, and once only. */ @Test - public void testInitialization() { + void testInitialization() { final Properties props = TestEngine.getTestProperties(); props.setProperty( CachingManager.PROP_CACHE_ENABLE, "true" ); props.setProperty( "jspwiki.pageProvider", "org.apache.wiki.providers.CounterProvider" ); final TestEngine engine = TestEngine.build( props ); - final CounterProvider p = (CounterProvider)((CachingProvider)engine.getManager( PageManager.class ).getProvider()).getRealProvider(); + final CounterProvider p = ( CounterProvider )( ( CachingProvider )engine.getManager( PageManager.class ).getProvider() ).getRealProvider(); Assertions.assertEquals( 1, p.m_initCalls, "init" ); Assertions.assertEquals( 1, p.m_getAllPagesCalls, "getAllPages" ); @@ -64,10 +62,12 @@ public class CachingProviderTest { engine.getManager( PageManager.class ).getPage( "Foo" ); Assertions.assertEquals( 0, p.m_pageExistsCalls, "pageExists2" ); + + engine.shutdown(); } @Test - public void testSneakyAdd() throws Exception { + void testSneakyAdd() throws Exception { final TestEngine engine = TestEngine.build(); final String dir = engine.getWikiProperties().getProperty( FileSystemProvider.PROP_PAGEDIR ); final File f = new File( dir, "Testi.txt" ); @@ -83,8 +83,34 @@ public class CachingProviderTest { final String text = engine.getManager( PageManager.class ).getText( "Testi"); Assertions.assertEquals( "[fuufaa]", text, "text" ); + engine.shutdown(); + } + + @Test + void testGetAllWithCacheTooSmallDelegatesToRealProvider() throws Exception { + final Properties props = TestEngine.getTestProperties(); + props.setProperty( CachingManager.PROP_CACHE_ENABLE, "true" ); + props.setProperty( "jspwiki.cache.config-file", "ehcache-jspwiki-small.xml" ); + + final TestEngine engine = TestEngine.build( props ); + engine.saveText( "page1", "page that should be cached" ); + engine.saveText( "page2", "page that should not be cached" ); + + Assertions.assertEquals( 2, engine.getManager( PageManager.class ).getAllPages().size() ); + engine.shutdown(); + } + + @Test + void testGetAllWithCacheTooSmallDelegatesToRealProviderWithInitialPageLoad() throws Exception { + final Properties props = TestEngine.getTestProperties(); + props.setProperty( CachingManager.PROP_CACHE_ENABLE, "true" ); + props.setProperty( "jspwiki.pageProvider", "org.apache.wiki.providers.CounterProvider" ); + props.setProperty( "jspwiki.cache.config-file", "ehcache-jspwiki-small.xml" ); + + final TestEngine engine = TestEngine.build( props ); - // TODO: ReferenceManager check as well + Assertions.assertEquals( 4, engine.getManager( PageManager.class ).getAllPages().size() ); + engine.shutdown(); } } diff --git a/jspwiki-main/src/test/resources/ehcache-jspwiki-small.xml b/jspwiki-main/src/test/resources/ehcache-jspwiki-small.xml new file mode 100644 index 000000000..331b26d01 --- /dev/null +++ b/jspwiki-main/src/test/resources/ehcache-jspwiki-small.xml @@ -0,0 +1,71 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> +<ehcache> + + <!-- Sets the path to the directory where cache .data files are created. + + If the path is a Java System Property it is replaced by + its value in the running VM. The following properties are translated: + + user.home - User's home directory + user.dir - User's current working directory + java.io.tmpdir - Default temp file path + --> + <diskStore path="java.io.tmpdir" /> + <!-- + The following attributes are required: + + maxElementsInMemory - Sets the maximum number of objects that will be created in memory + eternal - Sets whether elements are eternal. If eternal, timeouts are ignored and the + element is never expired. + overflowToDisk - Sets whether elements can overflow to disk when the in-memory cache + has reached the maxInMemory limit. + + The following attributes are optional: + timeToIdleSeconds - Sets the time to idle for an element before it expires. + i.e. The maximum amount of time between accesses before an element expires + Is only used if the element is not eternal. + Optional attribute. A value of 0 means that an Element can idle for infinity. + The default value is 0. + timeToLiveSeconds - Sets the time to live for an element before it expires. + i.e. The maximum time between creation time and when an element expires. + Is only used if the element is not eternal. + Optional attribute. A value of 0 means that and Element can live for infinity. + The default value is 0. + diskPersistent - Whether the disk store persists between restarts of the Virtual Machine. + The default value is false. + diskExpiryThreadIntervalSeconds- The number of seconds between runs of the disk expiry thread. The default value + is 120 seconds. + memoryStoreEvictionPolicy - Policy would be enforced upon reaching the maxElementsInMemory limit. Default + policy is Least Recently Used (specified as LRU). Other policies available - + First In First Out (specified as FIFO) and Less Frequently Used + (specified as LFU) + --> + + <!-- the default JSPWiki caches --> + <cache name="jspwiki.renderingCache" maxElementsInMemory="1" /> + <cache name="jspwiki.pageCache" maxElementsInMemory="1" /> + <cache name="jspwiki.pageTextCache" maxElementsInMemory="1" /> + <cache name="jspwiki.pageHistoryCache" maxElementsInMemory="1" /> + <cache name="jspwiki.attachmentsCache" maxElementsInMemory="1" /> + <cache name="jspwiki.attachmentCollectionsCache" maxElementsInMemory="1" /> + <cache name="jspwiki.dynamicAttachmentCache" maxElementsInMemory="1" /> + +</ehcache>
