397193 - Use a format easier to read for humans for the update.rdf

Status: RESOLVED WONTFIX

(Other Applications Graveyard :: McCoy, enhancement)

Description

[Johannes Walther]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a9pre) Gecko/2007092202 SeaMonkey/2.0a1pre
Build Identifier: 

It's quite hard for me to read the update.rdf generated by McCoy for two reasons.
Firstly the components for different extensions are mixed with another 
Maybe I should mention that I put all the infos for all my extensions into one update.rdf. I don't know if that's common but it should be respected.
Secondly currently McCoy writes four statements minimum (i.e. if an extension is only for one app, add one more statement for each additional app) to describe an update, linked together by meaningless (for humans) resource identifiers like rdf:#$VakZ52

Isn't it possible to write out or at least retain if it exists a structure like the following?

  <RDF:Description about="urn:mozilla:extension:{30c20af7-ec80-4b0f-3e9e-19643a93a784}">
    <em:updates>
      <RDF:Seq>
        <RDF:li resource="urn:mozilla:extension:{30c20af7-ec80-4b0f-3e9e-19643a93a784}:0.1" />
      </RDF:Seq>
    </em:updates>
  </RDF:Description>

  <RDF:Description about="urn:mozilla:extension:{30c20af7-ec80-4b0f-3e9e-19643a93a784}:0.1">
    <em:version>0.1</em:version>
    <em:targetApplication>
      <Description em:id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}"
        em:minVersion="1.5"
        em:maxVersion="3.0"
        em:updateLink="http://example.net/some.xpi"
        em:updateHash="sha256:49d2bcf7cf88703cc75e9e77a9a69b363b6be27ec6241795b20f51da2ae6159a" />
    </em:targetApplication>
  </RDF:Description>


Reproducible: Always

Comment 1

[Dave Townsend [:mossop]]

Yes this is something I'd like to do at some point.

Comment 2

[Justin Dolske [:Dolske]]

Attachment: Summary of problem

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 281965 [details]
Summary of problem

Thank you for your unique input dolske, maybe next time you could add something productive

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

I apologise for the size of this patch Axel, if you think you won't be able to get through it then I can try to find someone else, it is only for McCoy.

This switches away from using the core RDF APIs for manipulating the RDF files. Instead it adds a new js module that allows for loading an RDF file into a DOM document then provides an API allowing for querying and modifying the DOM document. The API is close to the core RDF. All of the functions in the module are commented for what they do. In addition the traditional assert/unassert style methods there are those made to treat predicates as single value properties which is common in the install.rdf and update.rdf.

The patch also contains surrounding mochitests and switches the McCoy code to use the new RDF module.

Comment 5

[Dave Townsend [:mossop]]

Attachment: try again

Oops, previous patch had some stuff from a different patch.

Comment 6

[Axel Hecht [:Pike]]

Here are some top-level comments:

I usually consider associating resource objects with datasource objects a bad idea. In my mind, resources are just unbound dots in the universe, and anybody can create any datasource to say anything about them.

I'm having a not-so-easy time in actually dissecting the API itself from the patch. When I was thinking about APIs lately, it has been more in a jqery/e4x style, but that's mostly query APIs.

http://hg.mozilla.org/users/axel_mozilla.com/rdf2/index.cgi/file/tip has that state. Nothing for write, granted.

Comment 7

[Dave Townsend [:mossop]]

Attachment: rough interface

JS objects, particularly involving inheritance, do get tricky to follow so attached is a rough idl for the module. Maybe it would be a good idea to see if you're ok with that first.

I don't necessarily have a problem with changing it so resources aren't linked to a datasource and so moving all the mutation methods onto the datasource, however the datasource would still internally need to maintain a mapping between a resource and it's associated Elements in the DOM document. Combine that with the fact that the way McCoy is going to be changing datasources is predominantly just setting properties for resources and I thought this might be the better way to go. Open to other ideas though.

Comment 8

[Dave Townsend [:mossop]]

Attachment: patch rev 2

This contains a few minor tweaks to the file reading and writing code. In some cases the files weren't getting flushed and closed fast enough for the mochitests on Linux and Windows.

Comment 9

[Axel Hecht]

I saw a few

 ["nodeID", "about", "resource", "ID", "parseType"].indexOf(attr.localName)

which I don't see working out. If I'm right, is there a way to get some test coverage for the amount of code?

Comment 10

[Dave Townsend [:mossop]]

Attachment: patch rev 3

I could only find the one case which you called out and you're right, that isn't correct. It was incorrectly adding RDF:nodeID as a literal on the resource. Fixed that and added testcases for it.

Comment 11

[Axel Hecht]

Not sure if there's any value in it, did you take a look at http://www.w3.org/TR/rdf-testcases/?

Comment 12

[Dave Townsend [:mossop]]

Attachment: ntriples rev 1

(In reply to comment #11)
> Not sure if there's any value in it, did you take a look at
> http://www.w3.org/TR/rdf-testcases/?
> 

You realise you've just made me write more code for you now ;)

This first patch removes the old rdf comparison function I wrote and replaces it with an ntriple based one. An rdf datasource is converted to a set of ntriples which are then compared line by line.

Unlike the previous code it has no restrictions on what rdf it can test, however it sacrifices speed for improved capabilities. Rather than try to guess which blank nodes match it just tests every possible mapping of blank nodes in one document to the other.

Given that this is just for tests I figure accuracy is better than speed.

Comment 13

[Dave Townsend [:mossop]]

Attachment: patch rev 4

This is an update of the rdf datasource code from running through the rdf testsuite. The main changes are:

Proper testing for XML Names for nodeID and ID attributes.
Throw an error when encountering invalid properties or subject types in the document.
Correctly create an rdf:type property for subjects when we can't create a prefix:localname to name the subject DOM node.
Deal with many issues around relative resource URI handling.

Comment 14

[Dave Townsend [:mossop]]

Attachment: rdf testsuite rev 1

This adds code to run the rdf testsuite tests, just the positive and negative parser tests for now. It requires a local copy of the testsuite with an altered manifest that hides tests we don't pass for now which is because of the following things that the code doesn't support:

No support for rdf:parseType (Literal or Resource)
No support for rdf:datatype
http://www.w3.org/TR/rdf-syntax-grammar/#section-Syntax-property-attributes-on-property-element
http://www.w3.org/TR/rdf-syntax-grammar/#section-Syntax-reifying
Missing rdf:RDF document element
Don't care if an rdf:ID is repeated in the same document
http://www.w3.org/TR/rdf-syntax-grammar/#section-Syntax-languages

I don't think any of these are really necessary to be implemented for now and this still leaves 97 positive parser and 40 negative parser tests all passing.

For some additional coverage I've also taken the expected result ntriples for the positive parser tests and used that to create the datasource, then test against the ntriples, then save and re-parse and test again.

Comment 15

[Axel Hecht]

Regarding attachment 330066 [details] [diff] [review], ntriples rev 1:

Factor the ntriples/bnode split in compareNTriples into a single method, that could very well be just internal to compareNTriples.

The mapGenerator isn't really a mapGenerator, AFAICT. I'd do something like

for (var thisBlankB in permutations(blanksB)) {...} 

and do some other renames to get rid of "map". It made at least me look for something different.

Triples is an array of strings, right? No idea if that'd be over-optimization, but I guess that using a hash on the subjects should speed things up a bit. But that's just a nit. Not sure how much of a perf pain this is and how often you actually run the tests ;-).

Comment 16

[Axel Hecht]

Comment on attachment 330066 [details] [diff] [review]
ntriples rev 1

r=me with comment 15

Comment 17

[Dave Townsend [:mossop]]

Attachment: ntriples rev 2

Ok, this takes the review comments, it also does the hashing by subject which is more efficient. Also took in some renames and some useful functions that were previously in later patches to make them a bit simpler.

Comment 18

[Dave Townsend [:mossop]]

Attachment: patch rev 5

Updated for the new N-Triple structure

Comment 19

[Dave Townsend [:mossop]]

Attachment: rdf testsuite rev 2

Updated to the new N-Triple structure

Comment 20

[Dave Townsend [:mossop]]

Comment on attachment 334756 [details] [diff] [review]
ntriples rev 2

These are basic changes to the test suite to allow converting rdf into n-triples form for comparing graphs

Comment 21

[Dave Townsend [:mossop]]

Comment on attachment 334757 [details] [diff] [review]
patch rev 5

This implements the RDF parser and serialiser, it keeps the full DOM tree around and mutates it as assertions are made.

Comment 22

[Dave Townsend [:mossop]]

Comment on attachment 334758 [details] [diff] [review]
rdf testsuite rev 2

This adds the W3 RDF test suite, or at least as much of it as the code supports.

Comment 23

[Axel Hecht]

Just looked that review list for this account, are these reviews still requested?

Comment 24

[Dave Townsend [:mossop]]

I think if this gets any more work it will be as a fork so reviews probably won't be too necessary now

Comment 25

[Dave Townsend [:mossop]]

I can no longer work on this project

Comment 26

[Sylvestre Ledru [:Sylvestre]]

McCoy is no longer maintained, closing.