You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jspwiki.apache.org by ju...@apache.org on 2020/02/24 16:52:44 UTC

[jspwiki] 02/38: JSPWIKI-120: rename + extract interface from AttachmentManager

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 205b397c70d5e48ce5df1293622b6c7aee7436eb
Author: juanpablo <ju...@apache.org>
AuthorDate: Wed Feb 19 14:25:00 2020 +0100

    JSPWIKI-120: rename + extract interface from AttachmentManager
---
 .../apache/wiki/attachment/AttachmentManager.java  | 505 +++------------------
 .../wiki/attachment/DefaultAttachmentManager.java  | 350 ++++++++++++++
 2 files changed, 420 insertions(+), 435 deletions(-)

diff --git a/jspwiki-main/src/main/java/org/apache/wiki/attachment/AttachmentManager.java b/jspwiki-main/src/main/java/org/apache/wiki/attachment/AttachmentManager.java
index 7391967..07dd4a5 100644
--- a/jspwiki-main/src/main/java/org/apache/wiki/attachment/AttachmentManager.java
+++ b/jspwiki-main/src/main/java/org/apache/wiki/attachment/AttachmentManager.java
@@ -18,35 +18,21 @@
  */
 package org.apache.wiki.attachment;
 
-import net.sf.ehcache.Cache;
-import net.sf.ehcache.CacheManager;
-import net.sf.ehcache.Element;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 import org.apache.wiki.WikiContext;
-import org.apache.wiki.WikiEngine;
 import org.apache.wiki.WikiPage;
 import org.apache.wiki.WikiProvider;
-import org.apache.wiki.api.exceptions.NoRequiredPropertyException;
 import org.apache.wiki.api.exceptions.ProviderException;
 import org.apache.wiki.api.exceptions.WikiException;
-import org.apache.wiki.pages.PageManager;
-import org.apache.wiki.parser.MarkupParser;
 import org.apache.wiki.providers.WikiAttachmentProvider;
-import org.apache.wiki.util.ClassUtil;
-import org.apache.wiki.util.TextUtil;
 
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Date;
 import java.util.List;
-import java.util.Properties;
 
 
 /**
@@ -57,152 +43,35 @@ import java.util.Properties;
  *
  *  @since 1.9.28
  */
-public class AttachmentManager
-{
-    /**
-     *  The property name for defining the attachment provider class name.
-     */
-    public static final String  PROP_PROVIDER = "jspwiki.attachmentProvider";
-
-    /**
-     *  The maximum size of attachments that can be uploaded.
-     */
-    public static final String  PROP_MAXSIZE  = "jspwiki.attachment.maxsize";
+public interface AttachmentManager {
 
-    /**
-     *  A space-separated list of attachment types which can be uploaded
-     */
-    public static final String PROP_ALLOWEDEXTENSIONS    = "jspwiki.attachment.allowed";
+    /** The property name for defining the attachment provider class name. */
+    String PROP_PROVIDER = "jspwiki.attachmentProvider";
 
-    /**
-     *  A space-separated list of attachment types which cannot be uploaded
-     */
-    public static final String PROP_FORBIDDENEXTENSIONS = "jspwiki.attachment.forbidden";
+    /** The maximum size of attachments that can be uploaded. */
+    String PROP_MAXSIZE  = "jspwiki.attachment.maxsize";
 
-    /**
-     *  A space-separated list of attachment types which never will open in the browser.
-     */
-    public static final String PROP_FORCEDOWNLOAD = "jspwiki.attachment.forceDownload";
+    /** A space-separated list of attachment types which can be uploaded */
+    String PROP_ALLOWEDEXTENSIONS = "jspwiki.attachment.allowed";
 
-    /** List of attachment types which are forced to be downloaded */
-    private String[] m_forceDownloadPatterns;
+    /** A space-separated list of attachment types which cannot be uploaded */
+    String PROP_FORBIDDENEXTENSIONS = "jspwiki.attachment.forbidden";
 
-    static Logger log = Logger.getLogger( AttachmentManager.class );
-    private WikiAttachmentProvider m_provider;
-    private WikiEngine             m_engine;
-    private CacheManager m_cacheManager = CacheManager.getInstance();
+    /** A space-separated list of attachment types which never will open in the browser. */
+    String PROP_FORCEDOWNLOAD = "jspwiki.attachment.forceDownload";
 
-    private Cache m_dynamicAttachments;
     /** Name of the page cache. */
-    public static final String CACHE_NAME = "jspwiki.dynamicAttachmentCache";
+    String CACHE_NAME = "jspwiki.dynamicAttachmentCache";
 
     /** The capacity of the cache, if you want something else, tweak ehcache.xml. */
-    public static final int   DEFAULT_CACHECAPACITY   = 1000;
-
-    /**
-     *  Creates a new AttachmentManager.  Note that creation will never fail,
-     *  but it's quite likely that attachments do not function.
-     *  <p>
-     *  <b>DO NOT CREATE</b> an AttachmentManager on your own, unless you really
-     *  know what you're doing.  Just use WikiEngine.getAttachmentManager() if
-     *  you're making a module for JSPWiki.
-     *
-     *  @param engine The wikiengine that owns this attachment manager.
-     *  @param props  A list of properties from which the AttachmentManager will seek
-     *  its configuration.  Typically this is the "jspwiki.properties".
-     */
-
-    // FIXME: Perhaps this should fail somehow.
-    public AttachmentManager( WikiEngine engine, Properties props )
-    {
-        String classname;
-
-        m_engine = engine;
-
-
-        //
-        //  If user wants to use a cache, then we'll use the CachingProvider.
-        //
-        boolean useCache = "true".equals(props.getProperty( PageManager.PROP_USECACHE ));
-
-        if( useCache )
-        {
-            classname = "org.apache.wiki.providers.CachingAttachmentProvider";
-        }
-        else
-        {
-            classname = props.getProperty( PROP_PROVIDER );
-        }
-
-        //
-        //  If no class defined, then will just simply fail.
-        //
-        if( classname == null )
-        {
-            log.info( "No attachment provider defined - disabling attachment support." );
-            return;
-        }
-
-        //
-        //  Create and initialize the provider.
-        //
-        String cacheName = engine.getApplicationName() + "." + CACHE_NAME;
-        try {
-            if (m_cacheManager.cacheExists(cacheName)) {
-                m_dynamicAttachments = m_cacheManager.getCache(cacheName);
-            } else {
-                log.info("cache with name " + cacheName + " not found in ehcache.xml, creating it with defaults.");
-                m_dynamicAttachments = new Cache(cacheName, DEFAULT_CACHECAPACITY, false, false, 0, 0);
-                m_cacheManager.addCache(m_dynamicAttachments);
-            }
-
-            Class<?> providerclass = ClassUtil.findClass("org.apache.wiki.providers", classname);
-
-            m_provider = (WikiAttachmentProvider) providerclass.newInstance();
-
-            m_provider.initialize(m_engine, props);
-        } catch( ClassNotFoundException e )
-        {
-            log.error( "Attachment provider class not found",e);
-        }
-        catch( InstantiationException e )
-        {
-            log.error( "Attachment provider could not be created", e );
-        }
-        catch( IllegalAccessException e )
-        {
-            log.error( "You may not access the attachment provider class", e );
-        }
-        catch( NoRequiredPropertyException e )
-        {
-            log.error( "Attachment provider did not find a property that it needed: "+e.getMessage(), e );
-            m_provider = null; // No, it did not work.
-        }
-        catch( IOException e )
-        {
-            log.error( "Attachment provider reports IO error", e );
-            m_provider = null;
-        }
-
-        String forceDownload = TextUtil.getStringProperty( props, PROP_FORCEDOWNLOAD, null );
-
-        if( forceDownload != null && forceDownload.length() > 0 )
-            m_forceDownloadPatterns = forceDownload.toLowerCase().split("\\s");
-        else
-            m_forceDownloadPatterns = new String[0];
-
-
-    }
+    int DEFAULT_CACHECAPACITY = 1_000;
 
     /**
      *  Returns true, if attachments are enabled and running.
      *
      *  @return A boolean value indicating whether attachment functionality is enabled.
      */
-    public boolean attachmentsEnabled()
-    {
-        return m_provider != null;
-    }
+    boolean attachmentsEnabled();
 
     /**
      *  Gets info on a particular attachment, latest version.
@@ -211,9 +80,7 @@ public class AttachmentManager
      *  @return Attachment, or null, if no such attachment exists.
      *  @throws ProviderException If something goes wrong.
      */
-    public Attachment getAttachmentInfo( String name )
-        throws ProviderException
-    {
+    default Attachment getAttachmentInfo( final String name ) throws ProviderException {
         return getAttachmentInfo( name, WikiProvider.LATEST_VERSION );
     }
 
@@ -226,11 +93,8 @@ public class AttachmentManager
      *  @throws ProviderException If something goes wrong.
      */
 
-    public Attachment getAttachmentInfo( String name, int version )
-        throws ProviderException
-    {
-        if( name == null )
-        {
+    default Attachment getAttachmentInfo( final String name, final int version ) throws ProviderException {
+        if( name == null ) {
             return null;
         }
 
@@ -238,18 +102,14 @@ public class AttachmentManager
     }
 
     /**
-     *  Figures out the full attachment name from the context and
-     *  attachment name.
+     *  Figures out the full attachment name from the context and attachment name.
      *
      *  @param context The current WikiContext
      *  @param attachmentname The file name of the attachment.
      *  @return Attachment, or null, if no such attachment exists.
      *  @throws ProviderException If something goes wrong.
      */
-    public Attachment getAttachmentInfo( WikiContext context,
-                                         String attachmentname )
-        throws ProviderException
-    {
+    default Attachment getAttachmentInfo( final WikiContext context, final String attachmentname ) throws ProviderException {
         return getAttachmentInfo( context, attachmentname, WikiProvider.LATEST_VERSION );
     }
 
@@ -258,140 +118,42 @@ public class AttachmentManager
      *
      *  @param context The current WikiContext
      *  @param attachmentname The file name of the attachment.
-     *  @return Attachment, or null, if no such attachment exists.
+     *  @param version A particular version.
+     *  @return Attachment, or null, if no such attachment or version exists.
      *  @throws ProviderException If something goes wrong.
      */
-    public String getAttachmentInfoName( WikiContext context,
-                                         String attachmentname )
-    {
-        Attachment att = null;
-
-        try
-        {
-            att = getAttachmentInfo( context, attachmentname );
-        }
-        catch( ProviderException e )
-        {
-            log.warn("Finding attachments failed: ",e);
-            return null;
-        }
-
-        if( att != null )
-        {
-            return att.getName();
-        }
-        else if( attachmentname.indexOf('/') != -1 )
-        {
-            return attachmentname;
-        }
-
-        return null;
-    }
+    Attachment getAttachmentInfo( WikiContext context, String attachmentname, int version ) throws ProviderException;
 
     /**
-     *  Figures out the full attachment name from the context and
-     *  attachment name.
+     *  Figures out the full attachment name from the context and attachment name.
      *
      *  @param context The current WikiContext
      *  @param attachmentname The file name of the attachment.
-     *  @param version A particular version.
-     *  @return Attachment, or null, if no such attachment or version exists.
-     *  @throws ProviderException If something goes wrong.
+     *  @return Attachment, or null, if no such attachment exists.
      */
-
-    public Attachment getAttachmentInfo( final WikiContext context,
-                                         String attachmentname,
-                                         final int version ) throws ProviderException {
-        if( m_provider == null ) {
-            return null;
-        }
-
-        WikiPage currentPage = null;
-
-        if( context != null ) {
-            currentPage = context.getPage();
-        }
-
-        //
-        //  Figure out the parent page of this attachment.  If we can't find it, we'll assume this refers directly to the attachment.
-        //
-        final int cutpt = attachmentname.lastIndexOf('/');
-
-        if( cutpt != -1 ) {
-            String parentPage = attachmentname.substring(0,cutpt);
-            parentPage = MarkupParser.cleanLink( parentPage );
-            attachmentname = attachmentname.substring(cutpt+1);
-
-            // If we for some reason have an empty parent page name;
-            // this can't be an attachment
-            if(parentPage.length() == 0) return null;
-
-            currentPage = m_engine.getPageManager().getPage( parentPage );
-
-            //
-            // Go check for legacy name
-            //
-            // FIXME: This should be resolved using CommandResolver,
-            //        not this adhoc way.  This also assumes that the
-            //        legacy charset is a subset of the full allowed set.
-            if( currentPage == null ) {
-                currentPage = m_engine.getPageManager().getPage( MarkupParser.wikifyLink( parentPage ) );
-            }
-        }
-
-        //
-        //  If the page cannot be determined, we cannot possibly find the attachments.
-        //
-        if( currentPage == null || currentPage.getName().length() == 0 ) {
-            return null;
-        }
-
-        //
-        //  Finally, figure out whether this is a real attachment or a generated attachment.
-        //
-        Attachment att = getDynamicAttachment( currentPage.getName()+"/"+attachmentname );
-
-        if( att == null ) {
-            att = m_provider.getAttachmentInfo( currentPage, attachmentname, version );
-        }
-
-        return att;
-    }
+    String getAttachmentInfoName( WikiContext context, String attachmentname );
 
     /**
-     *  Returns the list of attachments associated with a given wiki page.
-     *  If there are no attachments, returns an empty Collection.
+     *  Returns the list of attachments associated with a given wiki page. If there are no attachments, returns an empty Collection.
      *
      *  @param wikipage The wiki page from which you are seeking attachments for.
      *  @return a valid collection of attachments.
      *  @throws ProviderException If there was something wrong in the backend.
      */
-    public List< Attachment > listAttachments( WikiPage wikipage ) throws ProviderException {
-        if( m_provider == null ) {
-            return new ArrayList<>();
-        }
-
-        List< Attachment >atts = new ArrayList<>( m_provider.listAttachments( wikipage ) );
-        Collections.< Attachment >sort( atts, Comparator.comparing( Attachment::getName, m_engine.getPageManager().getPageSorter() ) );
-
-        return atts;
-    }
+    List< Attachment > listAttachments( WikiPage wikipage ) throws ProviderException;
 
     /**
-     *  Returns true, if the page has any attachments at all.  This is
-     *  a convinience method.
-     *
+     *  Returns true, if the page has any attachments at all.  This is a convenience method.
      *
      *  @param wikipage The wiki page from which you are seeking attachments for.
      *  @return True, if the page has attachments, else false.
      */
-    public boolean hasAttachments( WikiPage wikipage )
-    {
-        try
-        {
+    default boolean hasAttachments( final WikiPage wikipage ) {
+        try {
             return listAttachments( wikipage ).size() > 0;
+        } catch( final Exception e ) {
+            Logger.getLogger( AttachmentManager.class ).info( e.getMessage(), e );
         }
-        catch( Exception e ) {}
 
         return false;
     }
@@ -403,103 +165,49 @@ public class AttachmentManager
      *  @return true, if the attachment should be downloaded when clicking the link
      *  @since 2.11.0 M4
     */
-    public boolean forceDownload( String name )
-    {
-        if( name == null || name.length() == 0 ) return false;
-
-        name = name.toLowerCase();
-
-        if( name.indexOf('.') == -1) return true;  //force download on attachments without extension or type indication
-
-        for( int i = 0; i < m_forceDownloadPatterns.length; i++ )
-        {
-            if( name.endsWith(m_forceDownloadPatterns[i]) && m_forceDownloadPatterns[i].length() > 0 )
-                return true;
-        }
-
-        return false;
-    }
+    boolean forceDownload( String name );
 
     /**
-     *  Finds a (real) attachment from the repository as a stream.
+     *  Finds a (real) attachment from the repository as an {@link InputStream}.
      *
      *  @param att Attachment
-     *  @return An InputStream to read from.  May return null, if
-     *          attachments are disabled.
+     *  @return An InputStream to read from. May return null, if attachments are disabled.
      *  @throws IOException If the stream cannot be opened
      *  @throws ProviderException If the backend fails due to some other reason.
      */
-    public InputStream getAttachmentStream( Attachment att )
-        throws IOException,
-               ProviderException
-    {
+    default InputStream getAttachmentStream( final Attachment att ) throws IOException, ProviderException {
         return getAttachmentStream( null, att );
     }
 
     /**
-     *  Returns an attachment stream using the particular WikiContext.  This method
-     *  should be used instead of getAttachmentStream(Attachment), since it also allows
-     *  the DynamicAttachments to function.
+     *  Returns an attachment stream using the particular WikiContext. This method should be used instead of
+     *  {@link #getAttachmentStream(Attachment)}, since it also allows the DynamicAttachments to function.
      *
      *  @param ctx The Wiki Context
      *  @param att The Attachment to find
-     *  @return An InputStream.  May return null, if attachments are disabled.  You must
-     *          take care of closing it.
+     *  @return An InputStream.  May return null, if attachments are disabled.  You must take care of closing it.
      *  @throws ProviderException If the backend fails due to some reason
      *  @throws IOException If the stream cannot be opened
      */
-    public InputStream getAttachmentStream( WikiContext ctx, Attachment att )
-        throws ProviderException, IOException
-    {
-        if( m_provider == null )
-        {
-            return null;
-        }
-
-        if( att instanceof DynamicAttachment )
-        {
-            return ((DynamicAttachment)att).getProvider().getAttachmentData( ctx, att );
-        }
-
-        return m_provider.getAttachmentData( att );
-    }
-
-
+    InputStream getAttachmentStream( WikiContext ctx, Attachment att ) throws ProviderException, IOException;
 
     /**
-     *  Stores a dynamic attachment.  Unlike storeAttachment(), this just stores
-     *  the attachment in the memory.
+     *  Stores a dynamic attachment.  Unlike storeAttachment(), this just stores the attachment in the memory.
      *
      *  @param ctx A WikiContext
      *  @param att An attachment to store
      */
-    public void storeDynamicAttachment( WikiContext ctx, DynamicAttachment att )
-    {
-        m_dynamicAttachments.put(new Element(att.getName(), att));
-    }
+    void storeDynamicAttachment( WikiContext ctx, DynamicAttachment att );
 
     /**
-     *  Finds a DynamicAttachment.  Normally, you should just use getAttachmentInfo(),
-     *  since that will find also DynamicAttachments.
+     *  Finds a DynamicAttachment.  Normally, you should just use {@link #getAttachmentInfo(String)} , since that will find also
+     *  {@link DynamicAttachment}s.
      *
      *  @param name The name of the attachment to look for
      *  @return An Attachment, or null.
      *  @see #getAttachmentInfo(String)
      */
-
-    public DynamicAttachment getDynamicAttachment(String name) {
-        Element element = m_dynamicAttachments.get(name);
-        if (element != null) {
-            return (DynamicAttachment) element.getObjectValue();
-        } else {
-            //
-            //  Remove from cache, it has expired.
-            //
-            m_dynamicAttachments.put(new Element(name, null));
-
-            return null;
-        }
-    }
+    DynamicAttachment getDynamicAttachment( String name );
 
     /**
      *  Stores an attachment that lives in the given file. If the attachment did not exist previously, this method will create it.
@@ -511,7 +219,7 @@ public class AttachmentManager
      *  @throws IOException If writing the attachment failed.
      *  @throws ProviderException If something else went wrong.
      */
-    public void storeAttachment( final Attachment att, final File source ) throws IOException, ProviderException {
+    default void storeAttachment( final Attachment att, final File source ) throws IOException, ProviderException {
         try( final FileInputStream in = new FileInputStream( source ) ) {
             storeAttachment( att, in );
         }
@@ -523,29 +231,10 @@ public class AttachmentManager
      *
      *  @param att Attachment to store this under.
      *  @param in  InputStream from which the attachment contents will be read.
-     *
      *  @throws IOException If writing the attachment failed.
      *  @throws ProviderException If something else went wrong.
      */
-    public void storeAttachment( final Attachment att, final InputStream in ) throws IOException, ProviderException {
-        if( m_provider == null ) {
-            return;
-        }
-
-        //  Checks if the actual, real page exists without any modifications
-        //  or aliases.  We cannot store an attachment to a non-existent page.
-        if( !m_engine.getPageManager().pageExists( att.getParentName() ) ) {
-            // the caller should catch the exception and use the exception text as an i18n key
-            throw new ProviderException( "attach.parent.not.exist" );
-        }
-
-        m_provider.putAttachmentData( att, in );
-        m_engine.getReferenceManager().updateReferences( att.getName(), new ArrayList<>() );
-
-        final WikiPage parent = new WikiPage( m_engine, att.getParentName() );
-        m_engine.getReferenceManager().updateReferences( parent );
-        m_engine.getSearchManager().reindexPage( att );
-    }
+    void storeAttachment( Attachment att, InputStream in ) throws IOException, ProviderException;
 
     /**
      *  Returns a list of versions of the attachment.
@@ -556,45 +245,22 @@ public class AttachmentManager
      *          disabled.
      *  @throws ProviderException If the provider fails for some reason.
      */
-    public List< Attachment > getVersionHistory( final String attachmentName ) throws ProviderException {
-        if( m_provider == null ) {
-            return null;
-        }
-
-        final Attachment att = getAttachmentInfo( null, attachmentName );
-
-        if( att != null ) {
-            return m_provider.getVersionHistory( att );
-        }
-
-        return null;
-    }
+    List< Attachment > getVersionHistory( String attachmentName ) throws ProviderException;
 
     /**
-     *  Returns a collection of Attachments, containing each and every attachment
-     *  that is in this Wiki.
+     *  Returns a collection of Attachments, containing each and every attachment that is in this Wiki.
      *
-     *  @return A collection of attachments.  If attachments are disabled, will
-     *          return an empty collection.
+     *  @return A collection of attachments.  If attachments are disabled, will return an empty collection.
      *  @throws ProviderException If something went wrong with the backend
      */
-    public Collection<Attachment> getAllAttachments() throws ProviderException {
-        if( attachmentsEnabled() ) {
-            return m_provider.listAllChanged( new Date(0L) );
-        }
-
-        return new ArrayList<>();
-    }
+    Collection< Attachment > getAllAttachments() throws ProviderException;
 
     /**
      *  Returns the current attachment provider.
      *
      *  @return The current provider.  May be null, if attachments are disabled.
      */
-    public WikiAttachmentProvider getCurrentProvider()
-    {
-        return m_provider;
-    }
+    WikiAttachmentProvider getCurrentProvider();
 
     /**
      *  Deletes the given attachment version.
@@ -602,82 +268,51 @@ public class AttachmentManager
      *  @param att The attachment to delete
      *  @throws ProviderException If something goes wrong with the backend.
      */
-    public void deleteVersion( Attachment att )
-        throws ProviderException
-    {
-        if( m_provider == null ) return;
-
-        m_provider.deleteVersion( att );
-    }
+    void deleteVersion( Attachment att ) throws ProviderException;
 
     /**
      *  Deletes all versions of the given attachment.
+     *
      *  @param att The Attachment to delete.
      *  @throws ProviderException if something goes wrong with the backend.
      */
-    // FIXME: Should also use events!
-    public void deleteAttachment( Attachment att )
-        throws ProviderException
-    {
-        if( m_provider == null ) return;
-
-        m_provider.deleteAttachment( att );
-
-        m_engine.getSearchManager().pageRemoved( att );
-
-        m_engine.getReferenceManager().clearPageEntries( att.getName() );
-
-    }
+    void deleteAttachment( Attachment att ) throws ProviderException;
 
     /**
-     *  Validates the filename and makes sure it is legal.  It trims and splits
-     *  and replaces bad characters.
+     *  Validates the filename and makes sure it is legal.  It trims and splits and replaces bad characters.
      *
-     *  @param filename
+     *  @param filename file name to validate.
      *  @return A validated name with annoying characters replaced.
      *  @throws WikiException If the filename is not legal (e.g. empty)
      */
-    static String validateFileName( String filename )
-        throws WikiException
-    {
-        if( filename == null || filename.trim().length() == 0 )
-        {
-            log.error("Empty file name given.");
+    static String validateFileName( String filename ) throws WikiException {
+        if( filename == null || filename.trim().length() == 0 ) {
+            Logger.getLogger( AttachmentManager.class ).error( "Empty file name given." );
 
             // the caller should catch the exception and use the exception text as an i18n key
             throw new WikiException(  "attach.empty.file" );
         }
 
-        //
         //  Some browser send the full path info with the filename, so we need
         //  to remove it here by simply splitting along slashes and then taking the path.
-        //
-
-        String[] splitpath = filename.split( "[/\\\\]" );
+        final String[] splitpath = filename.split( "[/\\\\]" );
         filename = splitpath[splitpath.length-1];
 
-        //
-        //  Should help with IE 5.22 on OSX
-        //
+        // Should help with IE 5.22 on OSX
         filename = filename.trim();
 
         // If file name ends with .jsp or .jspf, the user is being naughty!
-        if( filename.toLowerCase().endsWith( ".jsp" ) || filename.toLowerCase().endsWith(".jspf") )
-        {
-            log.info( "Attempt to upload a file with a .jsp/.jspf extension.  In certain cases this " +
-                      "can trigger unwanted security side effects, so we're preventing it." );
-            //
+        if( filename.toLowerCase().endsWith( ".jsp" ) || filename.toLowerCase().endsWith( ".jspf" ) ) {
+            Logger.getLogger( AttachmentManager.class )
+                  .info( "Attempt to upload a file with a .jsp/.jspf extension.  In certain cases this " +
+                         "can trigger unwanted security side effects, so we're preventing it." );
+
             // the caller should catch the exception and use the exception text as an i18n key
             throw new WikiException(  "attach.unwanted.file"  );
         }
 
-        //
-        //  Remove any characters that might be a problem. Most
-        //  importantly - characters that might stop processing
-        //  of the URL.
-        //
-        filename = StringUtils.replaceChars( filename, "#?\"'", "____" );
-
-        return filename;
+        //  Remove any characters that might be a problem. Most importantly - characters that might stop processing of the URL.
+        return StringUtils.replaceChars( filename, "#?\"'", "____" );
     }
+
 }
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
new file mode 100644
index 0000000..4ac6c5d
--- /dev/null
+++ b/jspwiki-main/src/main/java/org/apache/wiki/attachment/DefaultAttachmentManager.java
@@ -0,0 +1,350 @@
+/*
+    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.
+ */
+package org.apache.wiki.attachment;
+
+import net.sf.ehcache.Cache;
+import net.sf.ehcache.CacheManager;
+import net.sf.ehcache.Element;
+import org.apache.log4j.Logger;
+import org.apache.wiki.WikiContext;
+import org.apache.wiki.WikiEngine;
+import org.apache.wiki.WikiPage;
+import org.apache.wiki.api.core.Engine;
+import org.apache.wiki.api.exceptions.NoRequiredPropertyException;
+import org.apache.wiki.api.exceptions.ProviderException;
+import org.apache.wiki.pages.PageManager;
+import org.apache.wiki.parser.MarkupParser;
+import org.apache.wiki.providers.WikiAttachmentProvider;
+import org.apache.wiki.references.ReferenceManager;
+import org.apache.wiki.search.SearchManager;
+import org.apache.wiki.util.ClassUtil;
+import org.apache.wiki.util.TextUtil;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.List;
+import java.util.Properties;
+
+
+/**
+ *  Provides facilities for handling attachments.  All attachment handling goes through this class.
+ *  <p>
+ *  The AttachmentManager provides a facade towards the current WikiAttachmentProvider that is in use.
+ *  It is created by the WikiEngine as a singleton object, and can be requested through the WikiEngine.
+ *
+ *  @since 1.9.28
+ */
+public class DefaultAttachmentManager implements AttachmentManager {
+
+    /** List of attachment types which are forced to be downloaded */
+    private String[] m_forceDownloadPatterns;
+
+    private static final Logger log = Logger.getLogger( DefaultAttachmentManager.class );
+    private WikiAttachmentProvider m_provider;
+    private Engine m_engine;
+    private CacheManager m_cacheManager = CacheManager.getInstance();
+    private Cache m_dynamicAttachments;
+
+    /**
+     *  Creates a new AttachmentManager.  Note that creation will never fail, but it's quite likely that attachments do not function.
+     *  <p><b>DO NOT CREATE</b> an AttachmentManager on your own, unless you really know what you're doing. Just use
+     *  WikiEngine.getAttachmentManager() if you're making a module for JSPWiki.
+     *
+     *  @param engine The wikiengine that owns this attachment manager.
+     *  @param props A list of properties from which the AttachmentManager will seek its configuration. Typically this is the "jspwiki.properties".
+     */
+    // FIXME: Perhaps this should fail somehow.
+    public DefaultAttachmentManager( final Engine engine, final Properties props ) {
+        m_engine = engine;
+
+        //  If user wants to use a cache, then we'll use the CachingProvider.
+        final boolean useCache = "true".equals( props.getProperty( PageManager.PROP_USECACHE ) );
+
+        final String classname;
+        if( useCache ) {
+            classname = "org.apache.wiki.providers.CachingAttachmentProvider";
+        } else {
+            classname = props.getProperty( PROP_PROVIDER );
+        }
+
+        //  If no class defined, then will just simply fail.
+        if( classname == null ) {
+            log.info( "No attachment provider defined - disabling attachment support." );
+            return;
+        }
+
+        //  Create and initialize the provider.
+        final String cacheName = engine.getApplicationName() + "." + CACHE_NAME;
+        try {
+            if( m_cacheManager.cacheExists( cacheName ) ) {
+                m_dynamicAttachments = m_cacheManager.getCache( cacheName );
+            } else {
+                log.info( "cache with name " + cacheName + " not found in ehcache.xml, creating it with defaults." );
+                m_dynamicAttachments = new Cache( cacheName, DEFAULT_CACHECAPACITY, false, false, 0, 0 );
+                m_cacheManager.addCache( m_dynamicAttachments );
+            }
+
+            final Class< ? > providerclass = ClassUtil.findClass( "org.apache.wiki.providers", classname );
+
+            m_provider = ( WikiAttachmentProvider )providerclass.newInstance();
+            m_provider.initialize( m_engine.adapt( WikiEngine.class ), props );
+        } catch( final ClassNotFoundException e ) {
+            log.error( "Attachment provider class not found",e);
+        } catch( final InstantiationException e ) {
+            log.error( "Attachment provider could not be created", e );
+        } catch( final IllegalAccessException e ) {
+            log.error( "You may not access the attachment provider class", e );
+        } catch( final NoRequiredPropertyException e ) {
+            log.error( "Attachment provider did not find a property that it needed: " + e.getMessage(), e );
+            m_provider = null; // No, it did not work.
+        } catch( final IOException e ) {
+            log.error( "Attachment provider reports IO error", e );
+            m_provider = null;
+        }
+
+        final String forceDownload = TextUtil.getStringProperty( props, PROP_FORCEDOWNLOAD, null );
+        if( forceDownload != null && forceDownload.length() > 0 ) {
+            m_forceDownloadPatterns = forceDownload.toLowerCase().split( "\\s" );
+        } else {
+            m_forceDownloadPatterns = new String[ 0 ];
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean attachmentsEnabled() {
+        return m_provider != null;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public String getAttachmentInfoName( final WikiContext context, final String attachmentname ) {
+        final Attachment att;
+        try {
+            att = getAttachmentInfo( context, attachmentname );
+        } catch( final ProviderException e ) {
+            log.warn( "Finding attachments failed: ", e );
+            return null;
+        }
+
+        if( att != null ) {
+            return att.getName();
+        } else if( attachmentname.indexOf( '/' ) != -1 ) {
+            return attachmentname;
+        }
+
+        return null;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public Attachment getAttachmentInfo( final WikiContext context, String attachmentname, final int version ) throws ProviderException {
+        if( m_provider == null ) {
+            return null;
+        }
+
+        WikiPage currentPage = null;
+
+        if( context != null ) {
+            currentPage = context.getPage();
+        }
+
+        //  Figure out the parent page of this attachment.  If we can't find it, we'll assume this refers directly to the attachment.
+        final int cutpt = attachmentname.lastIndexOf( '/' );
+        if( cutpt != -1 ) {
+            String parentPage = attachmentname.substring( 0, cutpt );
+            parentPage = MarkupParser.cleanLink( parentPage );
+            attachmentname = attachmentname.substring( cutpt + 1 );
+
+            // If we for some reason have an empty parent page name; this can't be an attachment
+            if( parentPage.length() == 0 ) {
+                return null;
+            }
+
+            currentPage = m_engine.getManager( PageManager.class ).getPage( parentPage );
+
+            // Go check for legacy name
+            // FIXME: This should be resolved using CommandResolver, not this adhoc way.  This also assumes that the
+            //        legacy charset is a subset of the full allowed set.
+            if( currentPage == null ) {
+                currentPage = m_engine.getManager( PageManager.class ).getPage( MarkupParser.wikifyLink( parentPage ) );
+            }
+        }
+
+        //  If the page cannot be determined, we cannot possibly find the attachments.
+        if( currentPage == null || currentPage.getName().length() == 0 ) {
+            return null;
+        }
+
+        //  Finally, figure out whether this is a real attachment or a generated attachment.
+        Attachment att = getDynamicAttachment( currentPage.getName() + "/" + attachmentname );
+        if( att == null ) {
+            att = m_provider.getAttachmentInfo( currentPage, attachmentname, version );
+        }
+
+        return att;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public List< Attachment > listAttachments( final WikiPage wikipage ) throws ProviderException {
+        if( m_provider == null ) {
+            return new ArrayList<>();
+        }
+
+        final List< Attachment > atts = new ArrayList<>( m_provider.listAttachments( wikipage ) );
+        atts.sort( Comparator.comparing( Attachment::getName, m_engine.getManager( PageManager.class ).getPageSorter() ) );
+
+        return atts;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean forceDownload( String name ) {
+        if( name == null || name.length() == 0 ) {
+            return false;
+        }
+
+        name = name.toLowerCase();
+        if( name.indexOf( '.' ) == -1 ) {
+            return true;  // force download on attachments without extension or type indication
+        }
+
+        for( final String forceDownloadPattern : m_forceDownloadPatterns ) {
+            if( name.endsWith( forceDownloadPattern ) && forceDownloadPattern.length() > 0 ) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public InputStream getAttachmentStream( final WikiContext ctx, final Attachment att ) throws ProviderException, IOException {
+        if( m_provider == null ) {
+            return null;
+        }
+
+        if( att instanceof DynamicAttachment ) {
+            return ( ( DynamicAttachment )att ).getProvider().getAttachmentData( ctx, att );
+        }
+
+        return m_provider.getAttachmentData( att );
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public void storeDynamicAttachment( final WikiContext ctx, final DynamicAttachment att ) {
+        m_dynamicAttachments.put( new Element( att.getName(), att ) );
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public DynamicAttachment getDynamicAttachment( final String name ) {
+        final Element element = m_dynamicAttachments.get( name );
+        if( element != null ) {
+            return ( DynamicAttachment )element.getObjectValue();
+        } else {
+            // Remove from cache, it has expired.
+            m_dynamicAttachments.put( new Element( name, null ) );
+            return null;
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public void storeAttachment( final Attachment att, final InputStream in ) throws IOException, ProviderException {
+        if( m_provider == null ) {
+            return;
+        }
+
+        // Checks if the actual, real page exists without any modifications or aliases. We cannot store an attachment to a non-existent page.
+        if( !m_engine.getManager( PageManager.class ).pageExists( att.getParentName() ) ) {
+            // the caller should catch the exception and use the exception text as an i18n key
+            throw new ProviderException( "attach.parent.not.exist" );
+        }
+
+        m_provider.putAttachmentData( att, in );
+        m_engine.getManager( ReferenceManager.class ).updateReferences( att.getName(), new ArrayList<>() );
+
+        final WikiPage parent = new WikiPage( m_engine, att.getParentName() );
+        m_engine.getManager( ReferenceManager.class ).updateReferences( parent );
+        m_engine.getManager( SearchManager.class ).reindexPage( att );
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public List< Attachment > getVersionHistory( final String attachmentName ) throws ProviderException {
+        if( m_provider == null ) {
+            return null;
+        }
+
+        final Attachment att = getAttachmentInfo( null, attachmentName );
+        if( att != null ) {
+            return m_provider.getVersionHistory( att );
+        }
+
+        return null;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public Collection<Attachment> getAllAttachments() throws ProviderException {
+        if( attachmentsEnabled() ) {
+            return m_provider.listAllChanged( new Date( 0L ) );
+        }
+
+        return new ArrayList<>();
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public WikiAttachmentProvider getCurrentProvider() {
+        return m_provider;
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public void deleteVersion( final Attachment att ) throws ProviderException {
+        if( m_provider == null ) {
+            return;
+        }
+
+        m_provider.deleteVersion( att );
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    // FIXME: Should also use events!
+    public void deleteAttachment( final Attachment att ) throws ProviderException {
+        if( m_provider == null ) {
+            return;
+        }
+
+        m_provider.deleteAttachment( att );
+        m_engine.getManager( SearchManager.class ).pageRemoved( att );
+        m_engine.getManager( ReferenceManager.class ).clearPageEntries( att.getName() );
+    }
+
+}