Commit 54d73d78 authored by Jody Garnett's avatar Jody Garnett
Browse files

Fix an infinite loop when file1.sld already exists [GEOS-6983]

Reported from Matthieu via: #970

* Cleaned up comments and exceptions
* make rename style aware of format
* tone down warnings from geoserver persister
* Add test linked to commit
* Fix an infinite loop when file1.sld already exists (variable i was never incremented)
parent 33c7f158
......@@ -6,6 +6,7 @@
package org.geoserver.config;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
......@@ -1202,7 +1203,7 @@ public class GeoServerDataDirectory implements ResourceStore {
protected @Nonnull Style parsedStyleResources(StyleInfo s) throws IOException {
final Resource styleResource = style(s);
if ( styleResource.getType() == Type.UNDEFINED ){
throw new IOException( "No such resource: " + s.getFilename());
throw new FileNotFoundException( "No such resource: " + s.getFilename());
}
final DefaultResourceLocator locator = new DefaultResourceLocator();
locator.setSourceUrl(resourceToUrl(styleResource));
......@@ -1356,6 +1357,9 @@ public class GeoServerDataDirectory implements ResourceStore {
});
}
catch(FileNotFoundException e){
GeoServerPersister.LOGGER.log(Level.WARNING, "Error loading style:"+ e);
}
catch(IOException e) {
GeoServerPersister.LOGGER.log(Level.WARNING, "Error loading style", e);
}
......
......@@ -19,6 +19,7 @@ import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.geoserver.catalog.Catalog;
import org.geoserver.catalog.CatalogException;
......@@ -30,8 +31,11 @@ import org.geoserver.catalog.LayerGroupInfo;
import org.geoserver.catalog.LayerInfo;
import org.geoserver.catalog.NamespaceInfo;
import org.geoserver.catalog.ResourceInfo;
import org.geoserver.catalog.SLDHandler;
import org.geoserver.catalog.StoreInfo;
import org.geoserver.catalog.StyleHandler;
import org.geoserver.catalog.StyleInfo;
import org.geoserver.catalog.Styles;
import org.geoserver.catalog.WMSLayerInfo;
import org.geoserver.catalog.WMSStoreInfo;
import org.geoserver.catalog.WorkspaceInfo;
......@@ -167,15 +171,19 @@ public class GeoServerPersister implements CatalogListener, ConfigurationListene
WorkspaceInfo newWorkspace = (WorkspaceInfo) event.getNewValues().get( i );
Resource newDir = dd.getStyles(newWorkspace);
//look for any resource files (image, etc...) and copy them over, don't move
// look for any resource files (image, etc...) and copy them over, don't move
// since they could be shared among other styles
for (Resource old : dd.additionalStyleResources((StyleInfo) source)) {
copyResToDir(old, newDir);
if (old.getType() != Type.UNDEFINED){
copyResToDir(old, newDir);
}
}
//move over the config file and the sld
for (Resource old : baseResources((StyleInfo)source)) {
moveResToDir(old, newDir);
if (old.getType() != Type.UNDEFINED){
moveResToDir(old, newDir);
}
}
}
......@@ -575,24 +583,54 @@ public class GeoServerPersister implements CatalogListener, ConfigurationListene
}
private void renameStyle( StyleInfo s, String newName ) throws IOException {
LOGGER.fine( "Renameing style " + s.getName() + " to " + newName );
LOGGER.fine( "Renaming style " + s.getName() + " to " + newName );
// rename xml configuration file
Resource xml = dd.config(s);
renameRes( xml, newName+".xml" );
// rename style definition file
Resource style = dd.style(s);
String sldFileName = newName + ".sld";
Resource target = style.parent().get(sldFileName);
int i = 1;
while(target.getType()!=Type.UNDEFINED && i <= MAX_RENAME_ATTEMPTS) {
sldFileName = newName + i + ".sld";
target = style.parent().get(sldFileName);
StyleHandler format = Styles.handler( s.getFormat() );
Resource target = uniqueResource( style, newName, format.getFileExtension() );
renameRes(style, target.name());
s.setFilename(target.name());
// rename generated sld if appropriate
if( !SLDHandler.FORMAT.equals(format.getFormat())){
Resource sld = style.parent().get(FilenameUtils.getBaseName(style.name()) + ".sld");
if( sld.getType() == Type.RESOURCE ){
Resource generated = uniqueResource( sld, newName, "sld" );
renameRes(sld, generated.name());
}
}
}
/**
* Determine unique name of the form <code>newName.extension</code>. newName will
* have a number appended as required to produce a unique resource name.
*
* @param resource Resource being renamed
* @param newName proposed name to use as a template
* @param extension extension
* @return New UNDEFINED resource suitable for use with rename
* @throws IOException If unique resource cannot be produced
*/
private Resource uniqueResource(Resource resource, String newName, String extension)
throws IOException {
Resource target = resource.parent().get(newName + "." + extension);
int i = 0;
while (target.getType() != Type.UNDEFINED && ++i <= MAX_RENAME_ATTEMPTS) {
target = resource.parent().get(newName + i + "." + extension);
}
if (i > MAX_RENAME_ATTEMPTS) {
throw new IOException("All target files between " + newName + "1.sld and " + newName
+ MAX_RENAME_ATTEMPTS + ".sld are in use already, giving up");
throw new IOException("All target files between " + newName + "1." + extension
+ " and " + newName + MAX_RENAME_ATTEMPTS + "." + extension
+ " are in use already, giving up");
}
renameRes(style, target.name());
s.setFilename(sldFileName);
return target;
}
private void modifyStyle( StyleInfo s ) throws IOException {
......
......@@ -571,6 +571,30 @@ public class GeoServerPersisterTest extends GeoServerSystemTestSupport {
assertThat( sldFile, not(fileExists()) );
assertThat( renamedSldFile, fileExists() );
}
@Test
public void testRenameStyleWithExistingIncrementedVersion() throws Exception {
testAddStyle();
File sldFile = new File(testData.getDataDirectoryRoot(), "styles/foostyle.sld");
sldFile.createNewFile();
File sldFile1 = new File(testData.getDataDirectoryRoot(), "styles/foostyle1.sld");
sldFile1.createNewFile();
File xmlFile1 = new File(testData.getDataDirectoryRoot(), "styles/foostyle1.xml");
xmlFile1.createNewFile();
StyleInfo s = catalog.getStyleByName("foostyle");
s.setName("foostyle");
catalog.save(s);
assertThat( sldFile, fileExists() );
assertThat( sldFile1, fileExists() );
assertThat( xmlFile1, fileExists() );
sldFile1.delete();
xmlFile1.delete();
}
@Test
public void testModifyStyleChangeWorkspace() throws Exception {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment