Refactoring the tag module

Development
#1

Hello,

In the context of https://jira.xwiki.org/browse/XWIKI-17606 I plan to integrate a sortable and paginated list of tagged pages using the live data macro, insted of the current list.
This live data will be populated using a tag specific LiveDataSource component.
But the current tag implementation is based on the deprecated plugins mechanism, and classes with static methods…

For this reason I propose to:

  1. move all the existing code to legacy
  2. introduce a script service instead of the plugin
  3. introduce a LiveDataSource
  4. replace the old static methods with interal components, supporting the implementation of the script service and live data source.

The use of $xwiki.tag will be replaced by $services.tag in the velocity scripts.

Below is the tentative signature of the new tag script service. It is inspired from the tag plugin where I have removed the operations unused in XS (i.e., getAllTags(), getTagCount(), getTagCountForQuery(String from, String where), and addTagToDocument(String tag, String documentName)), and replaced XWikiException by a new dedicated `TagException.

package org.xwiki.tag;

@Component
@Named("tag")
@Singleton
@Unstable
public class TagScriptService implements ScriptService
{
    /**
     * Get cardinality map of tags for a specific wiki space.
     *
     * @param space the space to get tags in
     * @return map of tags with their occurrences counts
     * @throws TagException if search query fails (possible failures: DB access problems, etc)
     */
    public Map<String, Integer> getTagCount(String space) throws TagException {}

    /**
     *
     * Get cardinality map of tags for list wiki spaces.
     *
     * @param spaces the list of space to get tags in, as a comma separated, quoted string
     * @return map of tags with their occurrences counts
     * @throws TagException if search query fails (possible failures: DB access problems, etc)
     */
    public Map<String, Integer> getTagCountForSpaces(String spaces) throws TagException {}

    /**
     * Get cardinality map of tags matching an hql query (parameterized version). Example of usage:
     * <ul>
     * <li><code>
     * $services.tag.getTagCountForQuery("", "doc.creator = :creator", {'creator' : "$!{request.creator}"})
     * </code> will return the cardinality map of tags for documents created by user-provided creator name</li>
     * </ul>
     *
     * @param from the from fragment of the query
     * @param where the parameterized where fragment from the query
     * @param parameters map of named parameters for the query
     * @return map of tags with their occurrences counts
     * @throws TagException if search query fails (possible failures: DB access problems, incorrect query
     *     fragments).
     */
    public Map<String, Integer> getTagCountForQuery(String from, String where, Map<String, ?> parameters)
        throws TagException {}

    /**
     * Get cardinality map of tags matching an hql query (parameterized version). Example of usage:
     * <ul>
     * <li><code>
     * $services.tag.getTagCountForQuery("", "doc.creator = ?1", ["$!{request.creator}"])
     * </code> will return the cardinality map of tags for documents created by user-provided creator name</li>
     * </ul>
     *
     * @param from the from fragment of the query
     * @param where the parameterized where fragment from the query
     * @param parameterValues list of parameter values for the query
     * @return map of tags with their occurrences counts
     * @throws TagException if search query fails (possible failures: DB access problems, incorrect query
     *     fragments).
     */
    public Map<String, Integer> getTagCountForQuery(String from, String where, List<?> parameterValues)
        throws TagException {}

    /**
     * Get all the documents containing the given tag.
     *
     * @param tag tag to match
     * @return list of pages
     * @throws TagException if search query fails (possible failures: DB access problems, etc)
     */
    public List<String> getDocumentsWithTag(String tag) throws TagException {}

    /**
     * Get tags from a document.
     *
     * @param documentName name of the document
     * @return list of tags
     * @throws TagException if document read fails (possible failures: insufficient rights, DB access problems,
     *     etc).
     */
    public List<String> getTagsFromDocument(String documentName) throws TagException {}

    /**
     * Add a list of tags to a document. The document is saved (minor edit) after this operation
     *
     * @param tags the comma separated list of tags to set; whitespace around the tags is stripped
     * @param documentName the name of the target document
     * @return the {@link TagOperationResult result} of the operation. {@link TagOperationResult#NO_EFFECT} is returned
     *     only if all the tags were already set on the document, {@link TagOperationResult#OK} is returned even if only
     *     some of the tags are new.
     */
    public TagOperationResult addTagsToDocument(String tags, String documentName) {}

    /**
     * Remove a tag from a document. The document is saved (minor edit) after this operation.
     *
     * @param tag tag to remove
     * @param documentName name of the document
     * @return the {@link TagOperationResult result} of the operation
     */
    public TagOperationResult removeTagFromDocument(String tag, String documentName) {}

    /**
     * Rename a tag in all the documents that contains it. Requires admin rights. Document containing this tag are saved
     * (minor edit) during this operation.
     *
     * @param tag tag to rename
     * @param newTag new tag
     * @return the {@link TagOperationResult result} of the operation
     */
    public TagOperationResult renameTag(String tag, String newTag) {}

    /**
     * Delete a tag from all the documents that contains it. Requires admin rights. Document containing this tag are
     * saved (minor edit) during this operation.
     *
     * @param tag tag to delete
     * @return the {@link TagOperationResult result} of the operation
     */
    public TagOperationResult deleteTag(String tag) {}
}

WDYT?

#2

You need to provide a pure Java solution too (ie no script, using a component). The ScriptService will then just delegate to that component. For example by introducing a TagManager component.

Since you’re designing an API you need to take into account all future needs too to avoid breaking the API to the max. So I think it’s a good idea to include all signatures you can imagine. And do it in a way that is extensible. For example (I’m not saying it’s a good thing, just showing what it could mean): getTaggedDocuments(DocumentQuery documentQuery). Then you implement a simple/basic DocumentQuery initially but allow extending the API in future by implementing new querying criteria.

It’s important that the API be extensible and not too restricive. Your API above seems quite restrictive. For example you have getTagsFromDocument(). What about getting the tags from a list of documents or from a document and all its nested docs, or from all docs in a wiki?

#3

That’s a pretty bad type to represent a list of spaces… I would suggest to not continue with horrible APIs like this and think of a better way to cover this use case.

As a general rule: use DocumentReference instead of String for this need. Also we already have a Document#getTagList() so not sure we really need this API ?