Hi,

Thanks for the feedback. There wasn't that much of problems with unit tests as 
I was afraid of.

When looking into the code I saw that these methods should have been in an 
AttributeDescriptorBuilder. I have been thinking more about it, and see that it 
could be using a AttributeTypeBuilder as a delegate. Then it wouldn't be that 
much work.

Another thing about AttributeTypeBuilder is that it's taken a new fluent 
approach, together with the more ordinary with setter's.
If we make a new AttributeDescriptorBuilder what would you prefer. Having it 
fluent like:

AttributeDescriptorBuilder builder = new AttributeDescriptorBuilder(factory); 
AttributeDescriptor attribute = 
builder.name("location").typeName("gml:PointPropertyType").binding(Point.class).buildDescriptor();

or

AttributeDescriptorBuilder builder = new AttributeDescriptorBuilder(factory); 
builder.setName("location");
builder.setTypeName("gml:PointPropertyType");
builder.setBinding(Point.class);
AttributeDescriptor attribute = builder.buildDescriptor();

Best regards,

Roar Brænden


> 27. mai 2022 kl. 03:33 skrev Jody Garnett <jody.garn...@gmail.com>:
> 
>  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
>  
> <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
>  
> <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 <andrea.a...@geosolutionsgroup.com 
> <mailto:andrea.a...@geosolutionsgroup.com>> 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 <jody.garn...@gmail.com 
> <mailto:jody.garn...@gmail.com>> 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 <roar.brenden...@gmail.com 
> <mailto:roar.brenden...@gmail.com>> 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 
> <mailto:GeoTools-Devel@lists.sourceforge.net>
> https://lists.sourceforge.net/lists/listinfo/geotools-devel 
> <https://lists.sourceforge.net/lists/listinfo/geotools-devel>
> -- 
> --
> Jody Garnett
> _______________________________________________
> GeoTools-Devel mailing list
> GeoTools-Devel@lists.sourceforge.net 
> <mailto:GeoTools-Devel@lists.sourceforge.net>
> https://lists.sourceforge.net/lists/listinfo/geotools-devel 
> <https://lists.sourceforge.net/lists/listinfo/geotools-devel>
> 
> 
> -- 
> Regards,
> Andrea Aime
> ==
> GeoServer Professional Services from the experts!
> Visit http://bit.ly/gs-services-us <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/ <https://www.geosolutionsgroup.com/>
> http://twitter.com/geosolutions_it <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
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to