Subject: [Geopriv] Review: draft-ietf-geopriv-loc-filters-04

From: "Thomson, Martin" <[email-masked]>
Date: Mon, 13 Jul 2009 22:44:46 -0500

This document is reasonably clear and concise.  It does however seem like a rush-job.  There are a lot of mistakes and it appears that crucial elements have not been thoroughly thought out.

(While looking into this, I discovered that the "pseudo-XPath" ABNF in RFC 4661 is rubbish.  It's possible to produce some pretty strange patterns.  I don't know how to interpret "//foo[******a:bc:de:fg//////=1]", but it's valid according to the ABNF.)

--

Section 2 - (nit) TriggerType is not an element, it's a type.  'trigger' is the element.

Section 3.1 attempts to use RFC 4661 to filter based on movement.  This does not work for a number of reasons:

  <changed by="x"> only works for numerical values.  pos is a multi-valued element.

  //pos[1] is not valid by the RFC 4661 ABNF.  This positional form of an XPath statement is not supported.

  Even if pos[1] were valid, it would not select the latitude component as described.  It would select the first pos element.  You need XPath 2.0 to do what you want, and the syntax would be "pos/item()[1]" or something to that effect (that seems right, but XPath 2.0 is a little light on examples in this area).

  The assumption of conversion to a point isn't valid.  The example shown can be processed by a RFC 4661 compliant processor without any knowledge of the content.  If knowledge of content is necessary, then generic RFC 4661 processing cannot be relied upon.

  Relying on change in latitude or longitude doesn't allow for someone to express a distance directly.  Having a means to specify a distance in metres is more useful and isn't subject to the vagaries that the proposed method has.

I conclude that a new type of filter is required for this use case.

Section 3.2 is reasonable.  With dynamic, there isn't any need for new mechanisms.  I'll note that the example and lead-in text don't match (20 or 3?).

 (nit) Would it be easier to construct examples that use //dyn:speed rather than the long XPath statement that is used here.  We already have sufficient guidance on how these elements are to be placed.

Section 3.3 is also OK, if potentially a little redundant.  (nit) Figure 3 is mis-labelled.  Figure 3 should also use RFC 5139 civic address, rather than RFC 4119.

Section 3.4 is good.  Figure 5 is labelled 3D, but the example is clearly 2D.  A 3D example would use a three-dimensional shape, such as a Prism; those are prohibited by text, so I see little reason to mention 2D or 3D at all.

Section 4 is no longer needed as it currently stands.  The text doesn't justify its own existence.  Previous (read: way back) versions of this document did well to justify the event rate throttling component.  Moving this to the related discussion in Section 2 seems to make sense.

Section 5 shouldn't use FeaturePropertyType.  GeometryPropertyType is probably more appropriate.  Rohan used features in initial versions, but this turns out to be a bad idea because you have too many extraneous elements and there is far too much flexibility in the definition of Features.  Geometry is most appropriate for this use anyhow.

Section 7 - I would argue that it's not necessary for a new MIME type.  RFC 4661 describes a perfectly adequate one (application/simple-filter+xml).  Extensions to that format do not need to define new MIME types.

Section 8 - You spelt Allan's surname incorrectly (mine is right, thanks).

Missing: geocoding/shape conversion use cases referenced in earlier email threads.

Cheers,
Martin

------------------------------------------------------------------------------------------------
This message is for the designated recipient only and may
contain privileged, proprietary, or otherwise private information.  
If you have received it in error, please notify the sender
immediately and delete the original.  Any unauthorized use of
this email is prohibited.
------------------------------------------------------------------------------------------------
[mf2]

---

From: "Hannes Tschofenig" <[email-masked]>
Date: Tue, 14 Jul 2009 11:21:31 +0300

Hi Martin, 

Thanks for the quick feedback. 

>This document is reasonably clear and concise.  It does 
>however seem like a rush-job.  There are a lot of mistakes and 
>it appears that crucial elements have not been thoroughly thought out.

It is a first shot as the structure of the document very much changed. It
was also very, very late. 
The alignment with RFC 4661 is essentially a document re-write that leaves a
number of questions open. 

>
>(While looking into this, I discovered that the "pseudo-XPath" 
>ABNF in RFC 4661 is rubbish.  It's possible to produce some 
>pretty strange patterns.  I don't know how to interpret 
>"//foo[******a:bc:de:fg//////=1]", but it's valid according to 
>the ABNF.)

I don't like Xpath nor do I like the version in RFC 4661. Unfortunately,
neither of the two are sufficient for the purpose we want. 

Actually, the entire sentence should be corrected since we currently only
have one element in there that extends the <trigger> element. 

>
>Section 3.1 attempts to use RFC 4661 to filter based on 
>movement.  This does not work for a number of reasons:
>
>  //pos[1] is not valid by the RFC 4661 ABNF.  This positional 
>form of an XPath statement is not supported.
>
>  Even if pos[1] were valid, it would not select the latitude 
>component as described.  It would select the first pos 
>element.  You need XPath 2.0 to do what you want, and the 
>syntax would be "pos/item()[1]" or something to that effect 
>(that seems right, but XPath 2.0 is a little light on examples 
>in this area).
>
>  The assumption of conversion to a point isn't valid.  The 
>example shown can be processed by a RFC 4661 compliant 
>processor without any knowledge of the content.  If knowledge 
>of content is necessary, then generic RFC 4661 processing 
>cannot be relied upon.
>
>  Relying on change in latitude or longitude doesn't allow for 
>someone to express a distance directly.  Having a means to 
>specify a distance in metres is more useful and isn't subject 
>to the vagaries that the proposed method has.
>

I also noticed some problems with this aspect. First, you are right with the
limitations of Xpath and RFC 4661 to point to a substring of the content of
an element. I did not look at Xpath 2.0 but that seemed to be necessary
given that RFC 4661 did not use Xpath either but a derived version of it. I
noted that in Section 3.1 of the document. 

Then, there is indeed the problem that looking at the individual values does
not provide you with the information you really want since it is the product
of the lat/long/alt value as a whole. 

Finally, the is the problem that RFC 4661 refers to XML documents and with
the large number of location shapes one cannot easily refer to a single XML
representation. In fact I had this problem already with all of the document
as there is still more than one option to present location despite the work
done in RFC 5491 as the GEOPRIV location information can be carried in a
<tuple>, <device>, and <person> element. I don't know whether it is
legitimate to just say in this document that one deals with virtual
"documents" where the values are always assumed to be in a specific place
(similarly to what I have done with the "virtual point" assumption for
Section 3.1.

I made a proposal for a simplification, see the link to the updated draft at
the end of the mail. 

>
>Section 3.2 is reasonable.  With dynamic, there isn't any need 
>for new mechanisms.

I agree; the downside is that pidf-lo dynamic became a normative reference.
Since pidf-lo dynamic is targed for experimental there will be a downref for
this document. I don't care too much about that but other folks might. 

  I'll note that the example and lead-in 
>text don't match (20 or 3?).

Typo. 

>
> (nit) Would it be easier to construct examples that use 
>//dyn:speed rather than the long XPath statement that is used 
>here.  We already have sufficient guidance on how these 
>elements are to be placed.

You mean something like:

<?xml version="1.0" encoding="UTF-8"?>
<filter-set xmlns="urn:ietf:params:xml:ns:simple-filter">
    <ns-bindings>
        <ns-binding prefix="dyn" 
            urn="urn:ietf:params:xml:schema:pidf:dynamic"/>        
    </ns-bindings>
    <filter id="123" uri="sip:[email-masked]">
        <trigger>
            <changed by="3">
              //dyn:speed
            </changed>
        </trigger>
    </filter>
</filter-set>

>
>Section 3.3 is also OK, if potentially a little redundant.  

I included it because it was listed as one of the things that was relevant
at the time of writing loc-filters. 
Quite useful, I think. 

>(nit) Figure 3 is mis-labelled.  Figure 3 should also use RFC 
>5139 civic address, rather than RFC 4119.

Good point. 

>
>Section 3.4 is good.  Figure 5 is labelled 3D, but the example 
>is clearly 2D.  A 3D example would use a three-dimensional 
>shape, such as a Prism; those are prohibited by text, so I see 
>little reason to mention 2D or 3D at all.

Brian's version allowed all shapes from RFC 5491; I thought that was a
complete overkill. 
So, I focused only on polygon and circle. 

Bug in the 3D example but it raises the question whether we really need a 3D
version at all. 

>
>Section 4 is no longer needed as it currently stands.  The 
>text doesn't justify its own existence.  Previous (read: way 
>back) versions of this document did well to justify the event 
>rate throttling component.  Moving this to the related 
>discussion in Section 2 seems to make sense.

True. 

>
>Section 5 shouldn't use FeaturePropertyType.  
>GeometryPropertyType is probably more appropriate.  Rohan used 
>features in initial versions, but this turns out to be a bad 
>idea because you have too many extraneous elements and there 
>is far too much flexibility in the definition of Features.  
>Geometry is most appropriate for this use anyhow.

I actually think that we only need to put the <polygon> and <circle> element
in there. 

Btw, I removed the containment schema that Rohan put in there that actually
extended PIDF-LO. I thought that this was outside the scope of this document
in any case. 

Any thoughts on that?

>
>Section 7 - I would argue that it's not necessary for a new 
>MIME type.  RFC 4661 describes a perfectly adequate one 
>(application/simple-filter+xml).  Extensions to that format do 
>not need to define new MIME types.

That's a good point. I will remove it. 

>Section 8 - You spelt Allan's surname incorrectly (mine is 

Good catch. 

>Missing: geocoding/shape conversion use cases referenced in 

Yes, I haven't incorporated the feedback you provided in 
http://www.ietf.org/mail-archive/web/geopriv/current/msg07432.html

Working on it. 

Here is the updated version: 
http://www.tschofenig.priv.at/svn/Filters/draft-ietf-geopriv-loc-filters-05.
txt

Ciao
Hannes

>_______________________________________________
>Geopriv mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/geopriv
>

---

From: "Thomson, Martin" <[email-masked]>
Date: Tue, 14 Jul 2009 17:58:40 -0500

Hi Hannes,

As always, you're very quick to update :)	

I think that you caught most of the problems.

You've still got the wrong namespace for civic, and the example could be simplified:

   <filter-set xmlns="urn:ietf:params:xml:ns:simple-filter">
       <ns-bindings>
           <ns-binding prefix="ca"
                 urn="urn:ietf:params:xml:ns:pidf:geopriv10:civicAddr"/>
       </ns-bindings>
       <filter id="ca123" uri="sip:[email-masked]">
           <trigger>
               <changed>//ca:A1</changed>
               <changed>//ca:A2</changed>
               <changed>//ca:A3</changed>
               <changed>//ca:PC</changed>
           </trigger>
       </filter>
   </filter-set>

(Note that I've changed the ID, since that can't start with a digit.  I haven't validated this, but I expect that it's good enough.)

I don't think that the schema validates as is (UPA again).  I seriously think that GeometryPropertyType is OK.  I would also extend xs:integer to allow for extensions (as attributes).  ...Or would xs:double be more flexible?  integer seems like it could be too restrictive, especially for those doing feet->metres conversions:

  ...
  <xs:element name="enterOrExit" type="gml:GeometryPropertyType"/>

  <xs:element name="moved" type="filter:movedType">
  <xs:complexType name="movedType">
    <xs:simpleContent>
      <xs:extension base="xs:double">
        <xs:anyAttribute namespace="##any" processContents="lax"/>
      </xs:extension>   
    </xs:simpleContent>
  </xs:complexType>

Aside from that, I'm now happy with the document.  I could care less about the down-ref and the containment schema seems unnecessary - if it turns out to be useful, then we can write another draft.

--Martin

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On
> Behalf Of Hannes Tschofenig
> Sent: Tuesday, 14 July 2009 6:22 PM
> To: Thomson, Martin; [email-masked]
> Subject: Re: [Geopriv] Review: draft-ietf-geopriv-loc-filters-04
> 
> Hi Martin,
>

---

From: "Hannes Tschofenig" <[email-masked]>
Date: Wed, 15 Jul 2009 10:02:28 +0300

Thanks for the quick response. 

I will cover it in my next update. 

Ciao
Hannes

>From: Thomson, Martin [mailto:[email-masked]] 
>Sent: 15 July, 2009 01:59
>To: Hannes Tschofenig; [email-masked]
>Subject: RE: [Geopriv] Review: draft-ietf-geopriv-loc-filters-04
>
>Hi Hannes,
>
>(Note that I've changed the ID, since that can't start with a 
>digit.  I haven't validated this, but I expect that it's good enough.)
>
>I don't think that the schema validates as is (UPA again).  I 
>seriously think that GeometryPropertyType is OK.  I would also 
>extend xs:integer to allow for extensions (as attributes).  
>...Or would xs:double be more flexible?  integer seems like it 
>could be too restrictive, especially for those doing 
>feet->metres conversions:
>
>  ...
>Aside from that, I'm now happy with the document.  I could 
>care less about the down-ref and the containment schema seems 
>unnecessary - if it turns out to be useful, then we can write 
>another draft.
>
>--Martin
>
>however seem 
>> >like a rush-job.  There are a lot of mistakes and it appears that 
>> >crucial elements have not been thoroughly thought out.
>> 
>> It is a first shot as the structure of the document very 
>much changed.
>> It
>> The alignment with RFC 4661 is essentially a document re-write that 
>> leaves a number of questions open.
>> 
>> >
>> >ABNF in RFC 4661 is rubbish.  It's possible to produce some pretty 
>> >strange patterns.  I don't know how to interpret 
>> >"//foo[******a:bc:de:fg//////=1]", but it's valid according to the 
>> >ABNF.)
>> 
>> Actually, the entire sentence should be corrected since we currently 
>> only have one element in there that extends the <trigger> element.
>> 
>> 
>> >
>> >Section 3.1 attempts to use RFC 4661 to filter based on movement.  
>> >This does not work for a number of reasons:
>> >
>element.  You 
>> >need XPath 2.0 to do what you want, and the syntax would be 
>> >"pos/item()[1]" or something to that effect (that seems right, but 
>> >XPath 2.0 is a little light on examples in this area).
>> >
>> >  The assumption of conversion to a point isn't valid.  The example 
>> >shown can be processed by a RFC 4661 compliant processor 
>without any 
>> >knowledge of the content.  If knowledge of content is 
>necessary, then 
>> >generic RFC 4661 processing cannot be relied upon.
>> >
>specify a 
>> >distance in metres is more useful and isn't subject to the vagaries 
>> >that the proposed method has.
>> >
>> I also noticed some problems with this aspect. First, you are right 
>> with the limitations of Xpath and RFC 4661 to point to a 
>substring of 
>> the content of an element. I did not look at Xpath 2.0 but 
>that seemed 
>> to be necessary given that RFC 4661 did not use Xpath either but a 
>> derived version of it. I noted that in Section 3.1 of the document.
>> 
>> Then, there is indeed the problem that looking at the individual 
>> values does not provide you with the information you really 
>want since 
>> it is the product of the lat/long/alt value as a whole.
>> 
>> Finally, the is the problem that RFC 4661 refers to XML 
>documents and 
>> with the large number of location shapes one cannot easily 
>refer to a 
>> single XML representation. In fact I had this problem 
>already with all 
>> of the document as there is still more than one option to present 
>> location despite the work done in RFC 5491 as the GEOPRIV location 
>> information can be carried in a <tuple>, <device>, and <person> 
>> element. I don't know whether it is legitimate to just say in this 
>> document that one deals with virtual "documents" where the 
>values are 
>> always assumed to be in a specific place (similarly to what I have 
>> done with the "virtual point" assumption for Section 3.1.
>> 
>> I made a proposal for a simplification, see the link to the updated 
>> draft at the end of the mail.
>> 
>> >
>> >Section 3.2 is reasonable.  With dynamic, there isn't any need for 
>> >new mechanisms.
>> 
>> Since pidf-lo dynamic is targed for experimental there will be a 
>> downref for this document. I don't care too much about that 
>but other 
>> folks might.
>> 
>> >rather than the long XPath statement that is used here.  We already 
>> >have sufficient guidance on how these elements are to be placed.
>> 
>> You mean something like:
>> 
>> <?xml version="1.0" encoding="UTF-8"?> <filter-set 
>> xmlns="urn:ietf:params:xml:ns:simple-filter">
>>     <ns-bindings>
>>         <ns-binding prefix="dyn"
>> I included it because it was listed as one of the things that was 
>> relevant at the time of writing loc-filters.
>> Quite useful, I think.
>> 
>> >Section 3.4 is good.  Figure 5 is labelled 3D, but the example is 
>> >clearly 2D.  A 3D example would use a three-dimensional shape, such 
>> >as a Prism; those are prohibited by text, so I see little reason to 
>> >mention 2D or 3D at all.
>> 
>> Bug in the 3D example but it raises the question whether we really 
>> need a 3D version at all.
>> 
>> >
>> >Section 4 is no longer needed as it currently stands.  The text 
>> >doesn't justify its own existence.  Previous (read: way
>> >back) versions of this document did well to justify the event rate 
>> >throttling component.  Moving this to the related discussion in 
>> >Section 2 seems to make sense.
>> 
>> True.
>> 
>> 
>> 
>> >
>> >features in initial versions, but this turns out to be a bad idea 
>> >because you have too many extraneous elements and there is far too 
>> >much flexibility in the definition of Features.
>> I actually think that we only need to put the <polygon> and <circle> 
>> element in there.
>> 
>> Btw, I removed the containment schema that Rohan put in there that 
>> actually extended PIDF-LO. I thought that this was outside the scope 
>> of this document in any case.
>> 
>> Any thoughts on that?
>> 
>> 
>> >
>> >Section 7 - I would argue that it's not necessary for a new MIME 
>> >type.  RFC 4661 describes a perfectly adequate one 
>> >(application/simple-filter+xml).  Extensions to that format do not 
>> >need to define new MIME types.
>> 
>> >This message is for the designated recipient only and may contain 
>> >privileged, proprietary, or otherwise private information.
>> >immediately and delete the original.  Any unauthorized use of this 
>> >email is prohibited.