Re: [xwiki-devs] [Proposal][Model] ResourceName refactoring

Should there be a ResourceType.OBJECT?

If I were to redesign the core, I would make objects able to have "child" objects. That way Document, Attachment, Space and even Wiki could extend Object. It might be a lofty ideal but I think we ought to make our APIs so they would be compatible if we were to go that route.

Caleb Thomas Mortagne wrote: > On Tue, Dec 15, 2009 at 20:39, Thomas Mortagne > <thomas.mortagne(a)xwiki.com> wrote: >> On Tue, Dec 15, 2009 at 19:02, Vincent Massol <vincent(a)massol.net> >> wrote: >>> On Dec 15, 2009, at 6:39 PM, Thomas Mortagne wrote: >>> >>>> On Tue, Dec 15, 2009 at 15:40, Vincent Massol <vincent(a)massol.net> >>>> wrote: >>>>> Hi, >>>>> >>>>> I'd like to propose a refactoring for >>>>> org.xwiki.model.DocumentName/ >>>>> AttachmentName. >>>>> >>>>> There are currently 2 problems with the current implementation: >>>>> - DocumentName doesn't support nested spaces (we need that for >>>>> the >>>>> future) >>>>> - We need to generalize the concept of resource names so that >>>>> we can >>>>> use the generic concept in the Model in some APIs >>>>> >>>>> Thus I'd like to propose: >>>>> >>>>> enum ResourceType >>>>> - WIKI, DOCUMENT, SPACE, ATTACHMENT >>>>> >>>>> ResourceName >>>>> - ResourceName(String name, ResourceName parent) >>>>> - get/setName() >>>>> - get/setParent(ResourceName) >>>>> - get/setType(ResourceType) >>>>> >>>>> DocumentName extends ResourceName >>>>> - DocumentName(String pageName, SpaceName parent) >>>>> >>>>> AttachmentName extends ResourceName >>>>> - AttachmentName(String fileName, DocumentName parent) >>>>> >>>>> WikiName extends ResourceName >>>>> - WikiName(String wikiName) >>>>> >>>>> SpaceName extends ResourceName >>>>> - SpaceName(String spaceName, SpaceName parent) >>>>> - SpaceName(String spaceName, WikiName parent) >>>>> >>>>> Open questions and comments >>>>> ======================== >>>>> >>>>> - Should we replace "Name" by "Reference", i.e. DocumentReference >>>>> instead of DocumentName, WikiReference instead of WikiName? >>>> "Reference" or "Path" or something like that yes since it's not >>>> really a name. >>>> >>>>> - Note: A name (or reference) isn't resistant to change. >>>>> Resources >>>>> (Document, Space, Wiki, etc) must also have an Identifier >>>>> (unique id) >>>>> to uniquely identify them. For example a Document can be moved >>>>> from >>>>> one space to another (the DocumentName changes in this case). >>>>> - The scheme above allows to map this easily to the JCR API >>>>> - Do we want helper methods for locating the wiki in which a >>>>> DocumentName is? That would mean adding: >>>>> WikiName DocumentName.getWiki() (algo: getParent() till getType >>>>> == >>>>> WIKI or null) >>>>> Same question for getting the last Space or Wiki from >>>>> AttachmentName >>>> Seems useful yes, we are using it a lot in the current >>>> DocumentName. >>>> >>>>> - Do we want a helper constructor to make it easier to create a >>>>> DocumentName? With the proposal above it means: >>>>> new DocumentName("page", new SpaceName("space", new >>>>> WikiName("wiki"))); >>>>> A helper constructor could be: DocumentName(String page, String >>>>> space, >>>>> String wiki). >>>> I don't think it's really necessary. We can always add it latter. >>>> >>>>> Problems: a) we would need another constructor to support a >>>>> list of >>>>> spaces and b) if there are fields other than the name to set on >>>>> ResourceNames later on, it's not going to work as smoothly) >>>>> >>>>> Factory and Serializer >>>>> ================= >>>>> >>>>> This is a big question I haven't yet solved. We have 2 options: >>>>> 1) have specialized Factory/Serializer. For ex: >>>>> DocumentNameFactory, >>>>> DocumentNameSerializer >>>>> 2) have generic ones: ResourceNameFactory/ResourceNameSerializer >>>>> >>>>> 2) seems nicer initially but the problem with 2) is that we >>>>> need a >>>>> global String representation of any resource in the system. >>>>> There are >>>>> 2 problems with that: >>>>> - we may not want that. For example when the user is asked to >>>>> enter a >>>>> document name in a field, we may not need/want to parse this as a >>>>> general resource that could for example point to an attachment >>>>> (and >>>>> btw as a consequence allow entering "@" which is our separator >>>>> for >>>>> attachments) >>>>> - it's very hard to add new Resource types later on (since we'd >>>>> need >>>>> more special characters to separate them and this means these >>>>> chars >>>>> wouldn't be allowed in names for pages for ex) >>>> We need escaping support anyway, we can't continue to use a syntax >>>> that does not support all characters, it introduce important >>>> limitations for no reason. In a real syntax the special >>>> characters has >>>> nothing to do with the supported content... >>> The factory cannot know if it needs to escape the '@' char (for e >>> ex). >>> So it would the code handling the form (for ex) that would need to >>> escape characters depending on what it is expecting (it would >>> need to >>> escape '@' for ex if it's expecting a document name and not >>> escape it >>> if it's expecting an attachment name). > > I think there is a misunderstanding, if what you mean is that > ResourceNameFactory is too generic to know '@' syntax i don't see > why. > The resources types are an enum and not generic string in your > proposal so ResourceNameFactory knows very well all the types and can > decide to have different separators character for each element. > >> The factory does not escape anything. A factory take a string in >> some >> syntax (syntax including escaping syntax) and parse it to create an >> object where all the part of the names are separated and unescaped. >> You have to give it a proper string, i don't see what it has to do >> with HTML form. >> >> I you want the factory to not take care of the attachment syntax >> part >> then you use ResourceNameFactory.create(String stringRepresentation, >> ResourceType.DOCUMENT) you put in your proposal... >> >>> The multi-factory solution removes this problem. >> Multi-factory does not bring any value, it's the opposite. It will >> just generate more classes and code duplicate. >> >>> Thanks >>> -Vincent >>> >>>>> The other option is to have 2) but with the type passed as >>>>> parameter: >>>>> ResourceName ResourceNameFactory.create(String >>>>> stringRepresentation, >>>>> ResourceType) >>>>> >>>>> However this simply means that ResourceNameFactory would be a >>>>> factory >>>>> for actual factories (DocumentNameFactory, AttachmentNameFactory, >>>>> etc) >>>>> so I don't really see the added value in our component-based >>>>> world >>>>> (we >>>>> can look up the right factory directly). >>>> In this case I don't see the point of having lots of different >>>> components interfaces just to support this ResourceType parameter. >>>> Plus: >>>> - when you need to write another (un)serializer syntax for >>>> resources >>>> (URIResourceNameSerializer for example) it's a lot less work/ >>>> classes >>>> - it's easier to handle escaping when you serialize in a central >>>> component like ResourceNameSerializer that knows all the >>>> syntaxes to >>>> escape. If you separate it in several different serializers >>>> WikiDocumentNameSerializer don't know it has to escape '@' for >>>> example >>>> since at its level attachment does not exists or you have to >>>> copy/past >>>> the code but with escaping of '@' in WikiAttachmentNameSerializer >>>> >>>>> Thus IMO we want 1). >>>> +1 for 2), -0 for 1) >>>> >>>>> WDYT? >>>>> >>>>> Thanks >>>>> -Vincent