Add a new XWikiUser.displayHiddenDocuments() API

Development
#1

Hi devs,

Right now our code uses 2 ways to find out if we should display hidden documents for the current user:

  • In most places: `Integer preference = userPreferencesSource.getProperty(“displayHiddenDocuments”, Integer.class);``
  • In one place: Object displayHiddenDocuments = documentAccessBridge.getProperty(parameters.user, userClass, "displayHiddenDocuments");. Note: this is needed because we don’t look for the current user’s preference but for a given user.

This is really not nice because it’s code duplication and it makes it harder to maintain it than it should. It also doesn’t follow the SOC principle (Separation of Concerns).

I’m thus proposing to add a new XWikiUser#displayHiddenDocuments() API.

Yes, I know that XWikiUser is in the com.xpn package and old, and we need a new proper User interface in the newer xwiki-platform-user-api module. However that’s harder to do and will take more time. I still think that doing what I propose is going in the right direction and will make it easier to switch from XWikiUser to any future User class.

WDYT?

PS: I think having a UserPreferencesConfigurationSource was actually a bad idea, because it’s too low level and doesn’t offer a nice typed API. Thus I think we should deprecate and legacify UserPreferencesConfigurationSource at some point. It becomes a liability since code can directly use it instead of going through XWikiUser. For example Simon recently added a XWikiUser#isDisabled() method but nothing prevents code from using directly UserPreferencesConfigurationSource and to check the “active” xproperty’s value. And yes, I know it’s possible to use the XWikiDocument API too to access a User profile’s properties too so that’s also a liability but this one is harder to remove :wink:

#2

ok there’s a problem to this proposal… :frowning:

It would mean depending on oldcore since XWikiUser is in oldcore…

#3

My use case was to implement https://jira.xwiki.org/browse/XWIKI-9762

FTM I’m proposing to intercept the call in UserPreferencesConfigurationSource by doing something such as:

@Component
@Named("user")
@Singleton
public class UserPreferencesConfigurationSource extends AbstractDocumentConfigurationSource
{
...
    @Override
    public <T> T getProperty(String key, T defaultValue)
    {
        // Treat the superadmin as a special case. Since we currently don't have a proper User API and since we want
        // the superadmin user to view hidden documents by default, we need to do it here.
        // TODO: See https://forum.xwiki.org/t/add-a-new-xwikiuser-displayhiddendocuments-api/6327 for a better
        // proposal.
        if ("displayHiddenDocuments".equals(key) && XWikiRightService.isSuperAdmin(getDocumentReference())) {
            return (T) defaultValue.getClass().cast(1);
        } else {
            return super.getProperty(key, defaultValue);
        }
    }
}

This is not great and keeps increasing the technical debt :frowning: but it seems we’re already quite deep in it when it concerns Users FTM.

I’m open to a better idea ofc.

Thanks

#4

I don’t see why you’re not going in that direction: it would be super helpful in lots of usecase and we don’t need to have a perfect API the first time. In any case I’ll be super happy to help for such API, especially if that would allow to solve https://jira.xwiki.org/browse/XWIKI-17012 too :slight_smile:

#5

For the same reason you didn’t do it when you added in 11.6RC1 in the XWikiUser class:

    /**
     * @param context used to retrieve the user document.
     * @return true if the user is disabled (i.e. its active property is set to 0). This always returns false if the
     *         user is the guest or superadmin user.
     * @since 11.6RC1
     */
    @Unstable
    public boolean isDisabled(XWikiContext context)

:slight_smile:

#6

Hmm pretty sure I’ve been discouraged at that moment to create another API because we already had an oldcore XWikUser :slight_smile: but quite frankly I’d really prefer if we had a clear abstraction to XWikiUser that wouldn’t imply to depend on oldcore. I had again the need for activitypub few days ago.

Now maybe I misunderstood and everyone would be happy to have such an API, if it’s the case I’ll add it myself.

#7

Yes we all would want to have it :slight_smile: Maybe there was a misunderstanding (I remember you asked and I remember replying but not saying we shouldn’t do it).

It just needs to:

  1. be good enough so that we don’t break backward compatibility in the future, or don’t expose it yet (internal package but then we cannot offer a public API to access it, which is ok if we go through a UserManager component. But should we? So it’s not just about deifning the new User class but also about how to access it. Note that we have a ModelManager which could also be a good candidate for returning it (it’s public though).
  2. we check what’s been done before to see if there’s something to salvage. I’m thinking about Jean-Vincent’s draft proposal about user api: https://design.xwiki.org/xwiki/bin/view/Design/UsersModule EDIT: I’ve checked it and there’s almost nothing in it, so nothing to get from it.
  3. we need to relatively quickly refactor our existing code so that we don’t continue to use the 3 old APIs (the XWikiUser one, the UserPreferencesConfigurationSource one or the direct access using the XWikiDocument API). At least mark the other one as deprecated. But that implies offering at least the same features as the 3 existing APIs… possibly not easy… The risk is just having yet another API to access user data, making it 4 APIs :wink:
#8

I’d be happy to work with you on this (time permitting).

#9

FYI I’ve started a basic implementation. Ping me if you want to share the work. I’ll publish something soon and create a design page.

#10

See https://forum.xwiki.org/t/new-basic-user-api/6329