Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.0
    • Fix Version/s: 2.3.0
    • Labels:
      None

      Description

      While removing the storeMail method from James (JAMES-392) and moving its logic in the LocalDelivery we could try to improve the configurability of the LocalDelivery.

      Here is a discussion on the mailinglist:

      > 1) Should I move the enableAliases/enableForward directly to the
      > LocalDelivery?

      Yes.
      I'm +1 for anything that allows configurations which were limited to being set once in James but used many times to being refactored to many to many, which means moving them nearer the point of use.

      Of course it also makes sense sometimes to retain a global default.

      And we've always applied the principle of least surprise when considering config questions, try to think through the questions that will arise when people upgrade, and take that into account when you make config file format changes. This usually means make it work by default like it did before unless the users choose to change it.

      > I'm not sure that trying to achieve backward compatibility in the
      > config.xml is a good idea (we would need a lot of junk in the code and
      > in the config to do that).

      I understand that. But work through what would be needed to break this, documentation etc is part of this consideration.

      > Please note that I already changed the place where you have to declare
      > mailetpackages/matcherpackages and the place you declare the main
      > spool repository.

      Yeah I noticed, you will have to be prepared to document this and support it on the users list when the questions flood in.

      > 2) IgnoreCase is instead used more than once and by more clients (Not
      > only
      > LocalDelivery) and so I was thinking that we should move this
      > configuration to the UserRepository interface. Should I do that?

      It is a global setting, the rfc (I forget) makes user parts case sensitive, we offer the choice to ignore this globally, anything else would (IMO) just be confusing.
      What benefit would the chnage bring?

      > 3) Should I remove the deprecated method storeMail from MailetContext now?
      > We also have a few other deprecated methods
      ...
      > previous release IMHO we can safely remove all of them: do you agree?

      Only if the next release is going to be a Major one (i.e. 3.x not 2.x)

      > 4) With this change I could split the LocalDelivery in 2 different new
      > mailets (leave LocalDelivery for backward compatibility) and create a
      > LocalUsersAliasingForwarding mailet that just apply aliasing and
      > forwarding for the mails processed and a LocalDelivery that simply
      > deliver the message to a repository named like the destination of the
      > message being processed (with no lookup on users). This would allow
      > much more flexible Configurations.

      +1 This is good, and paves the way for richer v'hosting.

      > 5) I can add configurations to the LocalDelivery (or the 2 new
      > mailets) in order to be able to use a different UserRepository
      > (different from the
      > LocalUsers) or a different inboxes repositories by allowing local
      > "overriding" configurations: what do you think?

      +1 Yes, this also is good, and also supports richer v'hosting support.

      (The third thing is to modify pop3 & imap handlers to implement combinations of IP address reverse dns and username naming convention to fudge that part.)

        Activity

        Hide
        bago Stefano Bagnara added a comment -

        I ran a performance test to know how much userinbox caching improve performance over a direct store.select (already caching).

        I done the test against 10000 user inboxes running a random get 1 million times.

        The differences between mailServer.getUserInbox (with local caching ReferenceMap) and the "manual" store.select (Configuration) (store already cache its results) is really small:
        9 seconds for the full run for getUserInbox
        12 seconds for the store.select.

        We could probably remove the getUserInbox method and leave to the store system and to its caching the whole handling.

        By now I think I will add configurability to the new ToMultiRepository in order to be able to specify a destinationUrl prefix and a type (MAIL/SPOOL).

        Eventually, in a second step, we could even add a strategy for repository naming (file://var/mail/#DOMAIN#/#LOCALPART#) or something similar. The same "startegy" could be added to pop3 configuration.

        Show
        bago Stefano Bagnara added a comment - I ran a performance test to know how much userinbox caching improve performance over a direct store.select (already caching). I done the test against 10000 user inboxes running a random get 1 million times. The differences between mailServer.getUserInbox (with local caching ReferenceMap) and the "manual" store.select (Configuration) (store already cache its results) is really small: 9 seconds for the full run for getUserInbox 12 seconds for the store.select. We could probably remove the getUserInbox method and leave to the store system and to its caching the whole handling. By now I think I will add configurability to the new ToMultiRepository in order to be able to specify a destinationUrl prefix and a type (MAIL/SPOOL). Eventually, in a second step, we could even add a strategy for repository naming ( file://var/mail/#DOMAIN#/#LOCALPART# ) or something similar. The same "startegy" could be added to pop3 configuration.
        Hide
        bago Stefano Bagnara added a comment -

        Also added default value for enableAliases and enableForwarding in #382481 (see JAMES-426)

        Show
        bago Stefano Bagnara added a comment - Also added default value for enableAliases and enableForwarding in #382481 (see JAMES-426 )
        Hide
        danny@apache.org Danny Angus added a comment -

        Closing issue fixed in released version.

        Show
        danny@apache.org Danny Angus added a comment - Closing issue fixed in released version.

          People

          • Assignee:
            bago Stefano Bagnara
            Reporter:
            bago Stefano Bagnara
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development