Hi Lenya devs,
some comments on the new DocumentManager.add() method:
General comment on the changes: the goals were to - ensure Lenya meta-data creation in all use cases - remove usage of java.io.File - document the process & make simplifications if possible
Frankly, I still haven't understood all of the existing code involved, so much of the functionality is exactly as it was before (just refactored). As admitted in my previous mail, I definitely agree there is room for improvement / clarification - for the time being I was happy to get the above goals working (I found it quite challenging to get something working at the same time in the default publication and in the blog publication)
> @param parentDocument The parent document.
This must be optional to allow creation of top-level documents. Should we allow to pass null objects? Then it should be noted in the javadoc comment.
We currently need the parent
- to construct the new URI for a default publication entry. So I guess for this the parent could be null, in which case we would simply use "/" as the base for the new entry (just committed that)
- to access the DocumentIdentityMap. Like previously said, I don't understand why the DocumentIdentityMap is accessed via an existing Document instance. If it were accessed via the Publication, we could pass that instead.
> @param newDocumentNodeName the name of the node representing the new > document
> @param newDocumentId the id of the new document
Why do we need both nodeName and documentId? Actually the document ID should be sufficient, that would even mean that the parent is unambigously defined.
The nodeName is used to build the new URI, whereas the newDocumentId is used to look up a new instance via the IdentityMap. I suppose these could be merged, but was looking understanding of the IdentityMap to risk it.
> @param documentTypeName the document type (aka resource type) of the > new document
Why don't we pass a DocumentType object? IMO OO code should use objects whenever possible and appropriate.
Sure, I think it would be just as valid to do it like that. The add() interface would be shorter, the code in the usecase would be longer; it's a tradeoff (as usual)
> @param language language of the new document
> @param navigationTitle navigation title
> @param initialContentsURI an URI from which initial contents for the > new document can be read. Optional parameter; may be set to > <code>null</code>, > in which case the default initial sample as configured in > <code>doctypes.xconf</code> will be used.
> @param nodeType the node type, as defined by the constants in [EMAIL PROTECTED] > org.apache.lenya.cms.authoring.NodeCreatorInterface > NodeCreatorInterface}
Do we really need the NodeType concept? What is it used for? Is it even possible to obtain the node type of a document through the API?
The blog entries use LEAF_NODE at creation; the default pub entries use BRANCH_NODE. I don't know if this has any implications.
> @param parameters any parameters the caller needs to pass to the creator
Can't we get rid of that? IMO it's a bad practise to allow polymorphism on the basis of arbitrary parameter sets.
It is in the creator interface, and used by the blog entry creator in the transformXML() method. I am not sure how the parameters would be passed otherwise ?
> @param useSiteManager set to true if the site manager is used in the
> publication; if set the site manager will be notified about the new
> document
IMO the site manager must be mandatory.
The problem is that publication.getSiteManagerHint() returns null when in the blog publication. I agree we should remove that parameter once the blog pub has a site manager.
Thx for your feedback & please make any improvements you feel are appropriate !
I lack understanding of the IdentityMap, site manager & blog pub internals to do them at this time. To me the important things for now are that Document.getResourceType() is reliable in all situations (so we can remove the DocumentTypeResolver) and that creation goes through lenya:// instead of File.
-- Wolfgang
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
