216788 - Clean up RDF API

Status: RESOLVED INCOMPLETE

(Core Graveyard :: RDF, defect)

Description

[Axel Hecht]

This is half-fallout from the rdf security review we had some day.

Issues to resolve:
- kill RDF resource factories (bug 211801)
- parsing and serialisation of RDF thru the RDF service

Should be:
- make Mark and Sweep noscript for file-datasource reloading, add Clear() instead.
- Separate out commands from nsIRDFDataSource, that's bookmarks only. I'd prefer
to have that on a separate interface, implemented by the bookmars service and
the composite datasource.
- Nuke js implementations of datasources? Basically making 
nsIRDFService::RegisterDataSource and UnregisterDataSource noscript. Or add
security checks. Data sources are services by concept, and webcontent can't
provide services. So the question is, should we support js impls of 
nsIDataSource?
If we do, we prolly want to move the two to nsPIRDFService. As that's called
in the callback from setting up the nsIRDFRemoteDataSource::Init() or related 
methods.

I'm not sure I have a good concept for parsing and serializing RDF in my head.
Loading is hard, too, esp load and error notification. The current interfaces
like nsIRDFXMLSink and nsIRDFXMLSinkObserver look pretty intermingled between
internal stuff and the mere "users should know when we're done".

And then there's still the huge intercaps issue with RDF. IIRC, it was voted to
not lower case the idls at the time when xpidl did that stuff, is that decision
still like that?

Comment 1

[Chase Tingley]

I agree that commands should be separated out into a separate interface
(I think it was alecf who proposed this first, in another bug).
I'm not in favor of axing support for JS nsIRDFDataSource implementations
though (I've used them in little projects), but some form or security
check would be appropriate.
The parsing and serializing interfaces are sort of garbled, and I'm
not sure the best way to untangle them.  As far as the code goes, I
have two patches in my tree to clean up parsing at the moment: one some
string work and one a revision to the container handling (half written),
but they don't really fall under the umbrella of this bug.

Comment 2

[Alec Flett]

its going to be hard to kill resource factories, but I'll take that up in the
other bug. I'm all in favor of moving commands elsewhere - I don't think that
was me but it really sounds like the right idea. This needs to be assigned to
the right owner though, rjc@netscape clearly isn't right.

Comment 3

[Alec Flett]

can someone put me on the CC for bug 211801?

Comment 4

[Axel Hecht]

The

 nsISimpleEnumerator GetElements();

in nsIRDFContainer doesn't specify wether it is working on a snapshot or on
the live container. This is causing some confusion down in bug 233238.

Comment 5

[Peter Wilson]

I know this is an old topic, but just to be on record. Disallowing JS
implementations of nsIDataSource is a really bad idea. I use them all the time
to wrap an existing datasource so as to add "computed" properties. For example,
I add a property to the file system datasource so that an Explorer-like view
sorts by directories and then files. 

Comment 6

[Alec Flett]

I would hope it doesn't come to that either.

Comment 7

[Axel Hecht]

Let's clear this up a bit.

Web-content must not implement singletons, so we have to hide the 
RegisterDataSource call from them.
I wonder if we really need that in the first place. As it really doesn't do 
anything we couldn't do inside the rdf service itself.
The only (edge) usecase I can see is that a service would register more than one
URL for it, but as the impl part of the rdf service calls GetService anyway, 
that should already work, if the component actually registers the right 
contract IDs.

I'd be fine with dropping that completely and not expose the cache of the rdf
service to the datasource impls.

Vetos out there, good contra usecases?

Comment 8

[Benjamin Smedberg]

Why do we have this singleton cache in the RDFservice at all? We don't need a
cache for most cases that I can think of, let's ditch it. People can call
getService directly and let XPCOM take care of it.

Comment 9

[Axel Hecht]

I wonder if any extensions rely on a config rdf/xml file in the profile being a 
singleton.
Or downloads.rdf, which may be more important.

Other than that, I could be fine with removing the singleton stuff and just add
a getNativeDataSource(AString aKey) to rdfPIService to serve as a non-web 
central entry point for datasources and somehow move the rdf/xml stuff into the 
parsing story as a separate item.
I'd like to have getNativeDataSource as a helper, so that you wouldn't have to
remember each tiny little detail about how the contract ids are made up. I could
very well live with that API not caching internally but just calling GetService
unconditionally.