Hi Jody, after the pull requests for [GEOS-10723] [GEOS-10724] and [GEOS-10743] accepted, we may also close the issues, doo. I went back to [10705] again and started another PR #6570<https://github.com/geoserver/geoserver/pull/6570>.
The more I dive into the Resource API the more questions arise. After reading the GSIPs about it, I think I should probably have made a separate GSIP from it. Solving the Resource.in() problem, I noticed, that calling Resorce.file() raises similar problems. Probably we need a hint whether the intention is to read the file or to create the file. But this really breaks the current API. I also just detected the jdbcstore community module. I'm currently about to understand it.... Then I think about the IllegalStateException thrown. This should possibly be an UncheckedIOException (since 1.8) which definitely covers the underlying FileNotFound Exception. Don't see if this concerns any error handling. And then the idea of using Optional or Consumer<InputStream> to hide the sensitive parts of dealing with the Inputstream. This may also improve many of the deeply nested error handlers. Regards, Dieter. Von: Jody Garnett <jody.garn...@gmail.com> Gesendet: Freitag, 21. Oktober 2022 21:58 An: Dieter Stüken - con terra GmbH <d.stue...@conterra.de> Cc: GeoServer <geoserver-devel@lists.sourceforge.net> Betreff: Re: [Geoserver-devel] (GEOS-10705) Loading missing resources creates unsolicited empty files 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<mailto: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<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgeoserver%2Fgeoserver%2Fpull%2F6216&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762577204%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=WS6HiPOGG7gg6E4SsnCg07apkqnWcU%2Bs04%2BSpwky9qQ%3D&reserved=0>. I think the root cause is the implementation of Resource.in() for UNDEFINED resources. Looking at Files.ResourceAdaptor.in<https://eur04.safelinks.protection.outlook.com/?url=http%3A%2F%2Ffiles.resourceadaptor.in%2F&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762577204%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=dIAvutYZgvYtC9nsJSkaEHLDqIJb2sVEOO8aj1Tl4wg%3D&reserved=0>() 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 d.stue...@conterra.de<mailto:d.stue...@conterra.de> con terra GmbH Martin-Luther-King-Weg 20<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.google.com%2Fmaps%2Fsearch%2FMartin-Luther-King-Weg%2B20%2B%250D%250A%2B48155%2BM%25C3%25BCnster%3Fentry%3Dgmail%26source%3Dg&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762577204%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=lGkmtaL5W5WHm32U9WtDe8dec%2FkwrSAK64nDJtfFPdY%3D&reserved=0> 48155 Münster<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.google.com%2Fmaps%2Fsearch%2FMartin-Luther-King-Weg%2B20%2B%250D%250A%2B48155%2BM%25C3%25BCnster%3Fentry%3Dgmail%26source%3Dg&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762577204%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=lGkmtaL5W5WHm32U9WtDe8dec%2FkwrSAK64nDJtfFPdY%3D&reserved=0> conterra.de<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.conterra.com%2F&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762732808%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=aT5OrCwdh1MiQqge9k%2FVkJI2wrISHQajELfcioCl2Mw%3D&reserved=0> Geschäftsführung: Karl Wiesmann, Uwe König HRB 4149, Amtsgericht Münster Privacy statements<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.con-terra.com%2Fprivacy-statements&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762732808%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=Rt770p58gYyd28xjGddi2GyWCxEnOrsAP6KKP%2B32kiM%3D&reserved=0> YouTube<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.youtube.com%2Fconterrachannel&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762732808%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=NxsyuaZeNWG%2B1iMaSr3bV1x0cc5as7nyotgOsZ6vVts%3D&reserved=0> | Twitter<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2Fconterra&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762732808%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=AC7o6gVa9OodCKBhJ22T2OBfkb7%2F%2BqyRWHrZL2UifvY%3D&reserved=0> | LinkedIn<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.linkedin.com%2Fcompany%2Fcon-terra-gmbh&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762732808%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=CcKrpneV3CfU%2FBTtF7ylBP6n5VnIT2Jj5i5VyrpnxTg%3D&reserved=0> _______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net<mailto:Geoserver-devel@lists.sourceforge.net> https://lists.sourceforge.net/lists/listinfo/geoserver-devel<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.sourceforge.net%2Flists%2Flistinfo%2Fgeoserver-devel&data=05%7C01%7C%7C2e008cb088634d28236808dab39e8a6a%7C6e0bfede3fcb4518a16565dc14fe5620%7C0%7C0%7C638019790762732808%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000%7C%7C%7C&sdata=Ghl%2F65FNsgDxlxsisQkicGhBQtKTFrtguFKi8Qyiqtk%3D&reserved=0> -- -- Jody Garnett
_______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geoserver-devel
