Re: [Geotools-devel] Proposal for the HTTPResponse interface
Thanks for the proposal Roar: The GeoTools wiki is not restricted, we can consider setting that up as needed. If I understand correctly the proposal is: - boolean *isOK()*: returns true for HTTP status 200 - not quite sure if you want to consider some of the other success codes such as 201 here also? - Can/should this handle redirects? - InputStream *getErrorStream()*: provides the error response content when isOK() false - Unclear what this does if isOK() true? Do you wish to throw an exception to indicate error stream is not available? - InputStream *getResponseStream()*: provides the response when isOK() is true, or throws an IOException (presumably with the contents of the error response?) Glad your proposal includes examples: - Where we would parse a ServiceException: Before and after show different logic (the first assumes failure) - Where we can't handle the error response.: Does not show use of getErrorStream() Let me try and write an example that uses both getErrorStream() and getResponseStream() to see if I understand the proposal: try { if (httpResponse.isOK()) { return ImageIOExt.readBufferedImage(httpResponse.getResponseStream()); } else { if ("application/vnd.ogc.se_xml".equalsIgnoreCase(httpResponse.getContentType())) { throw parseXMLException(httpResponse.getErrorStream()); } else if (httpResponse.getContentType().toLowerCase().startsWith("text/")){ throw parseException(httpResponse.getErrorStream()); } else { throw new IOException("Server returned error on request"); } } finally { httpResponse.dispose(); } Feedback: - This is an HTTP Request, can we just see the response code? If we had a status code isOK() becomes making a check against Arrays.asList(200,201,203); - Your ticket examples with HTTP 400 and 415 requests do not seem to be addressed here? - Tempting to document httpResponse.getContentType() as returning lowercase to avoid requiring equalsIgnoreCase() and toLowerCase() be used by all client code ever - Having two method names is difficult, and it maybe easier for users of the API if you added httpResponse.getResponseStream(boolean allowsErrorResponse) - Even better would be httpResponse.getResponseStream(int httpStatusCode ) providing you make the status code available Putting these ideas together: try { if (httpResponse.getStatus() == 200) { return ImageIOExt.readBufferedImage(httpResponse.getResponseStream()); } if (httpResponse.getStatus() == 400) { if ("application/vnd.ogc.se_xml".equals(httpResponse.getContentType())) { String message = parseXmlException(httpResponse.getErrorStream(400)); throw new HttpException(400,message); } else if (httpResponse.getContentType().startsWith("text/")){ String message = parseTextException( httpResponse.getErrorStream(400)); throw new HttpException(400,message); } } throw new HttpException(httpResponse.getStatus(),"Couldn't get image of tile: " + tile.getId()); } finally { httpResponse.dispose(); } Food for thought. -- Jody Garnett On Wed, 25 May 2022 at 00:34, Roar Brænden wrote: > Hi, > > I have a Proposal for the gt-http library. More specific the HTTPResponse > interface. I followed the instructions at the Wiki page, and created a New > Page, but when I clicked Save Page I didn't get a Pull Request as the > instructions indicated. It seems like the Page was created and freely > visible. > > > https://github.com/geotools/geotools/wiki/HTTPResponse-handling-error-responses > > Could you please have a look, and maybe give some feedback? > > Best regards, > Roar Brænden > > > ___ > GeoTools-Devel mailing list > GeoTools-Devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/geotools-devel > ___ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
Re: [Geotools-devel] AttributeDescriptor reporting wrong type name
I remember annoying cite test style problems where tests were checking that an attribute named "location" was required to be a PointPropertyType. OGC 07-036r1 *Table 11* GML object element: gml:Point GML type: gml:PointType GML property type: gml:PointPropertyType *F.2.1.2.4 Default property types for gml object types* A default GML property type may be defined in a GML application schema for every GML object type defined in that GML application schema. The GML property type shall either use or inherit directly or indirectly from one of the property types specified in 7.2.3 or it shall be defined in accordance with the patterns specified in this subclause. Here is the simple feature logic in our codebase: - https://github.com/geotools/geotools/blob/main/modules/library/xml/src/main/java/org/geotools/gml/producer/FeatureTypeTransformer.java#L446 And the annoying case of "location" property: - https://github.com/geotools/geotools/blob/main/modules/library/sample-data/src/main/resources/org/geotools/test-data/xml/gml/feature.xsd#L33 -- Jody Garnett On Thu, 26 May 2022 at 09:07, Andrea Aime wrote: > I'm actually not sure we ever use the type name... just the descriptor > name. > I have faint memories about this, but I think the parallel was: > - descriptor is like an XML element name > - type is like the XML type backing the element > > They can have legitimately two different names, no? In theory the same > type could be shared across multiple descriptors > Don't think it happens with simple features, not sure if that's something > used by app-schema, it might if it's driven by an actual XML schema though?. > > Cheers > Andrea > > On Thu, May 26, 2022 at 5:49 PM Jody Garnett > wrote: > >> Please go ahead with the fix, now that GeoServer has the ability to >> redefine schema attributes (and I hope to add the ability to edit title / >> abstract) this information will be more widely used and relied on. >> >> Jody >> >> >> On Wed, May 25, 2022 at 11:32 PM Roar Brænden >> wrote: >> >>> Hi, >>> >>> I was reading some shapefiles when it came to my attention that >>> something was strange with the schema's reported: >>> >>> TS14_max identified extends >>> polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) >>> TS21_min identified extends >>> polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) >>> TS16_mc identified extends >>> polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) >>> >>> The problem is why does it repeat the name of the fields. After some >>> digging in the code, I realized that the problem is within: >>> org.geotools.feature.AttributeTypeBuilder.buildDescriptor(String name) >>> >>> public AttributeDescriptor buildDescriptor(String name) { >>> setName(name); < name isn't the type's >>> name. It's the attributes name >>> if (binding == null) >>> throw new IllegalStateException("No binding has been >>> provided for this attribute"); >>> if (crs != null || Geometry.class.isAssignableFrom(binding)) { >>> return buildDescriptor(name, buildGeometryType()); >>> } else { >>> return buildDescriptor(name, buildType()); >>> } >>> } >>> >>> According to Github that code hasn't been changed for years, so GeoTools >>> users must have become familiar with this "problem". Because it might not >>> be a real problem. I do believe most people are using getBinding() rather >>> than looking at what is returned by getName() when looking at an >>> AttributeType. >>> >>> I'm quite sure removing that setName(name) line will fix the problem, >>> but doing so will possibly end up in a lot of unit test failures like this >>> one: >>> >>> [ERROR] ImageMosaicReaderTest.granuleSourceTest:1437 >>> expected:<...ltiPolygon,location:[location,time:time,endtime:endtime,date:date,lowz:lowz,highz:highz,loww:loww,highw:highw])> >>> but >>> was:<...ltiPolygon,location:[String,time:Date,endtime:Date,date:String,lowz:Integer,highz:Integer,loww:Integer,highw:Integer])> >>> >>> >>> Should I go forward fixing this, or are people so used to this that they >>> don't bother? >>> >>> Best regards, >>> Roar Brænden >>> >>> ___ >>> GeoTools-Devel mailing list >>> GeoTools-Devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/geotools-devel >>> >> -- >> -- >> Jody Garnett >> ___ >> GeoTools-Devel mailing list >> GeoTools-Devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/geotools-devel >> > > > -- > > Regards, > > Andrea Aime > > == > GeoServer Professional Services from the experts! > > Visit http://bit.ly/gs-services-us for more information. > == > > Ing. Andrea Aime > @geowolf > Technical Lead > > GeoSolutions Group > phone: +39
Re: [Geotools-devel] AttributeDescriptor reporting wrong type name
I'm actually not sure we ever use the type name... just the descriptor name. I have faint memories about this, but I think the parallel was: - descriptor is like an XML element name - type is like the XML type backing the element They can have legitimately two different names, no? In theory the same type could be shared across multiple descriptors Don't think it happens with simple features, not sure if that's something used by app-schema, it might if it's driven by an actual XML schema though?. Cheers Andrea On Thu, May 26, 2022 at 5:49 PM Jody Garnett wrote: > Please go ahead with the fix, now that GeoServer has the ability to > redefine schema attributes (and I hope to add the ability to edit title / > abstract) this information will be more widely used and relied on. > > Jody > > > On Wed, May 25, 2022 at 11:32 PM Roar Brænden > wrote: > >> Hi, >> >> I was reading some shapefiles when it came to my attention that something >> was strange with the schema's reported: >> >> TS14_max identified extends >> polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) >> TS21_min identified extends >> polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) >> TS16_mc identified extends >> polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) >> >> The problem is why does it repeat the name of the fields. After some >> digging in the code, I realized that the problem is within: >> org.geotools.feature.AttributeTypeBuilder.buildDescriptor(String name) >> >> public AttributeDescriptor buildDescriptor(String name) { >> setName(name); < name isn't the type's >> name. It's the attributes name >> if (binding == null) >> throw new IllegalStateException("No binding has been provided >> for this attribute"); >> if (crs != null || Geometry.class.isAssignableFrom(binding)) { >> return buildDescriptor(name, buildGeometryType()); >> } else { >> return buildDescriptor(name, buildType()); >> } >> } >> >> According to Github that code hasn't been changed for years, so GeoTools >> users must have become familiar with this "problem". Because it might not >> be a real problem. I do believe most people are using getBinding() rather >> than looking at what is returned by getName() when looking at an >> AttributeType. >> >> I'm quite sure removing that setName(name) line will fix the problem, but >> doing so will possibly end up in a lot of unit test failures like this one: >> >> [ERROR] ImageMosaicReaderTest.granuleSourceTest:1437 >> expected:<...ltiPolygon,location:[location,time:time,endtime:endtime,date:date,lowz:lowz,highz:highz,loww:loww,highw:highw])> >> but >> was:<...ltiPolygon,location:[String,time:Date,endtime:Date,date:String,lowz:Integer,highz:Integer,loww:Integer,highw:Integer])> >> >> >> Should I go forward fixing this, or are people so used to this that they >> don't bother? >> >> Best regards, >> Roar Brænden >> >> ___ >> GeoTools-Devel mailing list >> GeoTools-Devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/geotools-devel >> > -- > -- > Jody Garnett > ___ > GeoTools-Devel mailing list > GeoTools-Devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/geotools-devel > -- Regards, Andrea Aime == GeoServer Professional Services from the experts! Visit http://bit.ly/gs-services-us for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions Group phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 333 8128928 https://www.geosolutionsgroup.com/ http://twitter.com/geosolutions_it --- Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia. This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail ___ GeoTools-Devel mailing list
Re: [Geotools-devel] AttributeDescriptor reporting wrong type name
Please go ahead with the fix, now that GeoServer has the ability to redefine schema attributes (and I hope to add the ability to edit title / abstract) this information will be more widely used and relied on. Jody On Wed, May 25, 2022 at 11:32 PM Roar Brænden wrote: > Hi, > > I was reading some shapefiles when it came to my attention that something > was strange with the schema's reported: > > TS14_max identified extends > polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) > TS21_min identified extends > polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) > TS16_mc identified extends > polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) > > The problem is why does it repeat the name of the fields. After some > digging in the code, I realized that the problem is within: > org.geotools.feature.AttributeTypeBuilder.buildDescriptor(String name) > > public AttributeDescriptor buildDescriptor(String name) { > setName(name); < name isn't the type's name. > It's the attributes name > if (binding == null) > throw new IllegalStateException("No binding has been provided > for this attribute"); > if (crs != null || Geometry.class.isAssignableFrom(binding)) { > return buildDescriptor(name, buildGeometryType()); > } else { > return buildDescriptor(name, buildType()); > } > } > > According to Github that code hasn't been changed for years, so GeoTools > users must have become familiar with this "problem". Because it might not > be a real problem. I do believe most people are using getBinding() rather > than looking at what is returned by getName() when looking at an > AttributeType. > > I'm quite sure removing that setName(name) line will fix the problem, but > doing so will possibly end up in a lot of unit test failures like this one: > > [ERROR] ImageMosaicReaderTest.granuleSourceTest:1437 > expected:<...ltiPolygon,location:[location,time:time,endtime:endtime,date:date,lowz:lowz,highz:highz,loww:loww,highw:highw])> > but > was:<...ltiPolygon,location:[String,time:Date,endtime:Date,date:String,lowz:Integer,highz:Integer,loww:Integer,highw:Integer])> > > > Should I go forward fixing this, or are people so used to this that they > don't bother? > > Best regards, > Roar Brænden > > ___ > GeoTools-Devel mailing list > GeoTools-Devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/geotools-devel > -- -- Jody Garnett ___ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
[Geotools-devel] AttributeDescriptor reporting wrong type name
Hi, I was reading some shapefiles when it came to my attention that something was strange with the schema's reported: TS14_max identified extends polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) TS21_min identified extends polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) TS16_mc identified extends polygonFeature(the_geom:MultiPolygon,ID1:ID1,AV_Time:AV_Time,Shape_Leng:Shape_Leng,Shape_Area:Shape_Area) The problem is why does it repeat the name of the fields. After some digging in the code, I realized that the problem is within: org.geotools.feature.AttributeTypeBuilder.buildDescriptor(String name) public AttributeDescriptor buildDescriptor(String name) { setName(name); < name isn't the type's name. It's the attributes name if (binding == null) throw new IllegalStateException("No binding has been provided for this attribute"); if (crs != null || Geometry.class.isAssignableFrom(binding)) { return buildDescriptor(name, buildGeometryType()); } else { return buildDescriptor(name, buildType()); } } According to Github that code hasn't been changed for years, so GeoTools users must have become familiar with this "problem". Because it might not be a real problem. I do believe most people are using getBinding() rather than looking at what is returned by getName() when looking at an AttributeType. I'm quite sure removing that setName(name) line will fix the problem, but doing so will possibly end up in a lot of unit test failures like this one: [ERROR] ImageMosaicReaderTest.granuleSourceTest:1437 expected:<...ltiPolygon,location:[location,time:time,endtime:endtime,date:date,lowz:lowz,highz:highz,loww:loww,highw:highw])> but was:<...ltiPolygon,location:[String,time:Date,endtime:Date,date:String,lowz:Integer,highz:Integer,loww:Integer,highw:Integer])> Should I go forward fixing this, or are people so used to this that they don't bother? Best regards, Roar Brænden ___ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel