Re: [Geotools-devel] Proposal for the HTTPResponse interface

2022-05-26 Thread Jody Garnett
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

2022-05-26 Thread Jody Garnett
 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

2022-05-26 Thread Andrea Aime
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

2022-05-26 Thread Jody Garnett
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

2022-05-26 Thread Roar Brænden
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