Introduce an author setter helper in XWikiDocument for the most common use case

Development
#1

Hi devs,

After some time of using the new DocumentAuthors API and while we are happy with the feature it brings (separating of right and display authors) it added quite some complexity for most common use cases for developers of extensions who are manipulating the XWikiDocument API (the handling of authors in the script Document API is mostly automated so that one is fine IMO).

  1. I propose that we kind of get back a bit from the “let’s move everything to a dedicated DocumentAuthors API only to be clean” but this time to see XWikiDocument#setAuthorReference(UserReference) as a more discoverable unique helper for the most common use case (setting both the display and right authors to the same thing and let the store update the content author depending on the content dirty flag) and if you have a more advanced need you can manipulate the DocumentAuthors directly.

  2. As a bonus question, I’m honestly hesitating between a. setAuthorReference and b. setAuthor as the later might feel more natural for someone searching to set the author of the document.

WDYT ?

Here is my +1 for 1..

I’m fine with both variations in 2. with a slight preference for setAuthor. I would have definitely chosen setAuthorReference if there was a corresponding getter, but it’s really not the point here, and I would be -1 for such a getter right now as it would not make much sense.

#2

I definitely believe that we have regressed a lot in API discoverability/usability for setting the document’s author. As an example, I was coding a contrib extension today and couldn’t find how to set the content author as all setContentAuthor APIs were marked deprecated. In the end I found that I had to call getAuthors() to set the authors which is completely backward…

So, with that in mind, I’m +1 to restore the direct setting of authors with set*Author*() APIs.

The questions I have in mind with your proposal are:

  1. I don’t like having 2 recommended APIs to do the same thing, especially in the same class.
  2. It’s honestly very hard to understand all the different “authors” and the javadocs are not helping at all (quite the opposite: when you read the javadoc for the effective metadata auhor, it says: The effective metadata author is the author responsible of any change but the content and you learn that even though you’re modifying only the content you need to set the metadata author and not the content author - which is set automatically when the content changes…). So it’s hard to understand the various author concepts and having an api that hides that complexity could potentially lead to bugs for the cases where it’s not right to call the new setAuthor() API. OTOH the current situation is even worse and even more error-prone :slight_smile:
    • BTW you’ve used the term “rights author” in your text above and I find that much simpler to understand than “effective metadata author” which means not much…

About the API name, ideally setAuthor() is more natural. But we’ve added lots of set*Reference in the past when the parameter was a reference and going back to removing the Reference prefix is going to be non-consistent. For example:

  • We had public void setAuthor(String author) and we added public void setAuthorReference(DocumentReference authorReference) instead of adding public void setAuthor(DocumentReference authorReference)
  • Same for public void setContentAuthor(String contentAuthor) → public void setContentAuthorReference(DocumentReference contentAuthorReference)

So I’m fine with setAuthor(UserReference) if we decide that from now on our best practice is to use the shorter api names and not prefix with Reference when we don’t have to.

Thanks for starting this thread.

#3

That’s not what I propose. My proposal is only to have a single setAuthor(UserReference) helper for cases where you don’t need to worry about the different kinds of authors.

I’m not proposing two different ways to do the same thing IMO. The idea is to have a helper for a very common use case and have getAuthors() based APIs be the advanced API.

#4

I think I understood that, but adding a helper is going to offer one more way for users to do the same thing (i.e. 2 APIs). Precisely the user will need to decide to do:

doc.getAuthors().setEffectiveMetadataAuthor(...);
doc.getAuthors().setOriginalMetadataAuthor(...);

OR

doc.setAuthor(...);
#5

Sure, but for me, it’s not really concurrent APIs, it’s more about abstraction levels.

#6

+1 for XWikiDocument#setAuthor(UserReference).

Thanks,
Marius

#7

+1 for setAuthorReference
+0 for setAuthor
+1 to not define the corresponding getter

#8

Hello, will this simplify cases where another user replaces a document’s author, but the content’s author will still be the old author?

#9

This case will be covered by this API too, yes. The content author is automatically set to the provided author but only if the content dirty flag is true (i.e. if the content was modified).

#10

Thanks for answer. I prefer option 1 +1 for this and +1 for setAuthor(stringValue)

#11

Totally forgot this proposal, so: 4 +1 for 1. and setAuthor seems to win over setAuthorReference.

Creating the jira issue and I will introduce that in 16.1.0RC1