It is worth proposing a refactoring here. The guideline during the api developement was to optimize for the most common patterns of File usage we found in the codebase; and utillity methods such as in() for common usage patterns. Where possible we provided method name compatibility so it would be easier to migrate code from File to Resource.
I was expecting far more input on this API when it was first introduced; your input is welcome now. For the specific example you show: - in() should not accidentally create a file for UNDEFINED - returning an empty input stream would be okay - less for client code to handle. I understand that failed parsing expression is not so fun - throwing UncheckedIOException is okay - concerned it would lead to a bunch of boiler plate code try / catch being introduced Since we have the whole codebase available it should be easy to access what this change would require and answer the concerns above. Whatever change is made I ask that the javadocs be clearly updated. -- Jody On Fri, Oct 21, 2022 at 12:03 PM Dieter Stüken - con terra GmbH via Geoserver-devel <geoserver-devel@lists.sourceforge.net> wrote: > Hi dev list, > > I frequently noticed, that Resource.in() just creates non existing files > and then returns an empty input stream. This almost ever causes further > problems like https://github.com/geoserver/geoserver/pull/6216. > > I think the root cause is the implementation of Resource.in() for > UNDEFINED resources. > Looking at Files.ResourceAdaptor.in() I find: > > final File actualFile = file(); > if (!actualFile.exists()) { > throw new IllegalStateException("Cannot access " + actualFile); > } > > Seems, the initial intention was to throw an exception, but since file() > creates the missing file, it does not work as intended. I analyzed the > former GSIP 106, but I did not find any hints about the expected behavior > here. > > This can be fixed easily and in think an Exception about a missing file is > much more expressive than a message from a failed content parser. > > Most use cases check for Type.RESOURCE in advance or have some elaborated > exception handlers and are thus unaffected. Some usages however try to > handle this condition later on by using inputStream.available() like > EchoParametersDao.getEchoParameters(in) does and will fail then. > > I'm working on a pull request to change the behavior this way, but this > causes adaptions to other related areas. Even SpringResourceAdaptor is > affected, since it relays on IOExcetion which have been wrapped into > IllegalStateException (and for which I think an UncheckedIOException is the > better choice). > > Other solutions are: > > return null (can be checked afterwards but not so nice) > > return an empty InputStream (Java 11 even has some: > InputStream.nullInputStream()) > > Do you think it is worth to propose a refactoring here or should I better > keep hands off? > > Dieter. > > -- > > *Dieter Stüken* > > Software Engineer > > Nature Environment and Resources > > > > T +49 251 59689 403 > > <https://www.google.com/maps/search/Martin-Luther-King-Weg+20+%0D%0A+48155+M%C3%BCnster?entry=gmail&source=g> > > d.stue...@conterra.de > > > > con terra GmbH > > Martin-Luther-King-Weg 20 > <https://www.google.com/maps/search/Martin-Luther-King-Weg+20+%0D%0A+48155+M%C3%BCnster?entry=gmail&source=g> > > 48155 Münster > <https://www.google.com/maps/search/Martin-Luther-King-Weg+20+%0D%0A+48155+M%C3%BCnster?entry=gmail&source=g> > > conterra.de <https://www.conterra.com/> > > > > Geschäftsführung: Karl Wiesmann, Uwe König > > HRB 4149, Amtsgericht Münster > > Privacy statements <https://www.con-terra.com/privacy-statements> > > > > YouTube <https://www.youtube.com/conterrachannel> | Twitter > <https://twitter.com/conterra> | LinkedIn > <https://www.linkedin.com/company/con-terra-gmbh> > > > _______________________________________________ > Geoserver-devel mailing list > Geoserver-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/geoserver-devel > -- -- Jody Garnett
_______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geoserver-devel
