Dieter Stüken (
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=6048e1a1ceccdd006a25ea04
) *created* an issue
GeoServer (
https://osgeo-org.atlassian.net/browse/GEOS?atlOrigin=eyJpIjoiY2JkMzc3ZTIxN2JlNDlmZGJkZTM3YzRjNzk1YjRhODQiLCJwIjoiaiJ9
) / Improvement (
https://osgeo-org.atlassian.net/browse/GEOS-10705?atlOrigin=eyJpIjoiY2JkMzc3ZTIxN2JlNDlmZGJkZTM3YzRjNzk1YjRhODQiLCJwIjoiaiJ9
) GEOS-10705 (
https://osgeo-org.atlassian.net/browse/GEOS-10705?atlOrigin=eyJpIjoiY2JkMzc3ZTIxN2JlNDlmZGJkZTM3YzRjNzk1YjRhODQiLCJwIjoiaiJ9
) Loading missing resources creates unsolicited empty files. (
https://osgeo-org.atlassian.net/browse/GEOS-10705?atlOrigin=eyJpIjoiY2JkMzc3ZTIxN2JlNDlmZGJkZTM3YzRjNzk1YjRhODQiLCJwIjoiaiJ9
)
Issue Type: Improvement Assignee: Unassigned Components: Main Created:
12/Oct/22 7:08 PM Environment:
Geoserver startup
Priority: Medium Reporter: Dieter Stüken (
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=6048e1a1ceccdd006a25ea04
)
While reading its configuration various config.xml files are located as a
Resource and then parsed by XStreamPersister.load(r.in()). If the resource does
not exist calling Resource.in() turns the missing resource into an empty file.
This causes successive exceptions.
One of these cases was addressed by GEOS-10694 (
https://osgeo-org.atlassian.net/browse/GEOS-10694 ) , but I think this is a
general problem here.
I find several locations, where a configuration is loaded from a resource. Some
are testing if the resource exists, others don’t.
GWCConfigPersister.loadConfig() (
https://github.com/geoserver/geoserver/blob/main/src/gwc/src/main/java/org/geoserver/gwc/config/GWCConfigPersister.java#L76
) try/catch GeoServerLoader.depersist() (
https://github.com/geoserver/geoserver/blob/main/src/main/src/main/java/org/geoserver/config/GeoServerLoader.java#L1100
) none XStreamServiceLoader.load() (
https://github.com/geoserver/geoserver/blob/main/src/main/src/main/java/org/geoserver/config/util/XStreamServiceLoader.java#L56
) if/else LoggingStartupContextListener.getLogging() (
https://github.com/geoserver/geoserver/blob/main/src/main/src/main/java/org/geoserver/logging/LoggingStartupContextListener.java#L115
) if/else GeoServerSecurityManager.loadConfig() (
https://github.com/geoserver/geoserver/blob/main/src/main/src/main/java/org/geoserver/security/GeoServerSecurityManager.java#L2702
) throws IOException GeoServerSecurityManager.loadConfigFile() (
https://github.com/geoserver/geoserver/blob/main/src/main/src/main/java/org/geoserver/security/GeoServerSecurityManager.java#L2711
) throws IOException
To handle these cases consistently I suggest introducing a general method
public <T> T load(Resource res, Class<T> clazz) throws IOException
to handle missing files either by returning null (which is handled correctly in
most cases) or throwing a FileNotFoundException (which again leads to confusing
catch/try blocks again).
There are many other (61) examples where Resource.in() is called unchecked or
handled by complicated catch/try blocks, like in:
JDBCConfigurationStorage.getJDBCDiskQuotaConfig() (
https://github.com/geoserver/geoserver/blob/main/src/gwc/src/main/java/org/geoserver/gwc/JDBCConfigurationStorage.java#L83
)
IMHO: I can’t figure out any situation, where creating an empty input file
really makes sense to me. Instead, some Exception should be thrown. Looking at
FileSystemResourceStore.in() (
https://github.com/dieterstueken/geoserver/blob/main/src/platform/src/main/java/org/geoserver/platform/resource/FileSystemResourceStore.java#L198
) I think this was indeed intended. However, using File actualFile = file()
already creates the file unintentionally. Using this.file instead should
restore the expected behavior and also leads to a more consistent stack trace.
(I would rather suggest UncheckedIOException instead of IllegalStateException ).
I even think about introducing some Optional<T> to replace lots of those
try/catch and if/else monsters by something simpler like:
resource.tryLoad(someLoader::load).orElse(someDefault)
Unfortunately, most of the load() functions throw checked exceptions 😟 .
(
https://osgeo-org.atlassian.net/browse/GEOS-10705#add-comment?atlOrigin=eyJpIjoiY2JkMzc3ZTIxN2JlNDlmZGJkZTM3YzRjNzk1YjRhODQiLCJwIjoiaiJ9
) Add Comment (
https://osgeo-org.atlassian.net/browse/GEOS-10705#add-comment?atlOrigin=eyJpIjoiY2JkMzc3ZTIxN2JlNDlmZGJkZTM3YzRjNzk1YjRhODQiLCJwIjoiaiJ9
)
Get Jira notifications on your phone! Download the Jira Cloud app for Android (
https://play.google.com/store/apps/details?id=com.atlassian.android.jira.core&referrer=utm_source%3DNotificationLink%26utm_medium%3DEmail
) or iOS (
https://itunes.apple.com/app/apple-store/id1006972087?pt=696495&ct=EmailNotificationLink&mt=8
) This message was sent by Atlassian Jira (v1001.0.0-SNAPSHOT#100208-
sha1:399748d )
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
