Skip to content

JAMES-2972 Incorrect attribute name in the mailet configuration#177

Closed
jtconsol wants to merge 4 commits intoapache:masterfrom
jtconsol:JAMES-2972
Closed

JAMES-2972 Incorrect attribute name in the mailet configuration#177
jtconsol wants to merge 4 commits intoapache:masterfrom
jtconsol:JAMES-2972

Conversation

@jtconsol
Copy link
Contributor

@jtconsol jtconsol commented Nov 14, 2019

This fixes JAMES-2972 by correcting all mailet's matcher attributes to match. It deactivates the WithPriority mailet and related matchers in the spring app's mailetcontainer.xml, since in the current state of the config, each and every mail will be dropped more or less silently.

Then I took the liberty to add two log messages on level DEBUG:

  1. When a mailet's match attribute is empty, it defaults to match="All" upon initializiation. When that happens, a log message is now emitted.
    Mailet WithPriority has no 'match' attribute. Defaulting to match all mails.

  2. A log message is now emitted when the Null mailet destroys a message.
    Null mailet is destroying mail Mail1573732431806-e461b5de-5567-4d21-bb9a-da193a791067 from MaybeSender{mailAddress=Optional[james@localhost]} for [nina@localhost]

There is the requirement noted in JAMES-2972 to align the spring app's and docker-run-spring-app's mailetcontainer.xml with each other. In the chat it was noted, that the docker-run-spring-app's version should be preferred. But that version is very terse and does not have comments at all. Should the verbose one really be ditched? It'd recommend it the other way around (besides properly formatting the file).


NB: There are a lot of mailetcontainer.xmls in the project, but none is as "documented" as the spring app one.

size  crc32    location

3906  54a74d69 server/mailet/mailetcontainer-api/src/main/resources/mailetcontainer.xml
3946  e63a173d server/container/cli-integration/src/test/resources/mailetcontainer.xml
4008  5d7a5513 server/container/guice/jpa-smtp-mariadb/sample-configuration/mailetcontainer.xml
4008  5d7a5513 server/container/guice/jpa-smtp/sample-configuration/mailetcontainer.xml
4090  3b631531 mpt/impl/smtp/cassandra-rabbitmq-object-storage/src/test/resources/mailetcontainer.xml
4090  3b631531 mpt/impl/smtp/cassandra/src/test/resources/mailetcontainer.xml
4209  77197879 dockerfiles/run/guice/jpa-smtp/destination/conf/mailetcontainer.xml
4233  b7604959 server/container/guice/memory-guice/sample-configuration/mailetcontainer.xml
4422  e1bd239a server/container/guice/cassandra-ldap-guice/src/test/resources/mailetcontainer.xml
4422  e1bd239a server/container/guice/cassandra-rabbitmq-ldap-guice/src/test/resources/mailetcontainer.xml
4484  b02a6ca0 server/protocols/webadmin-integration-test/src/test/resources/mailetcontainer.xml
4506  4898698d server/container/guice/jpa-smtp-mariadb/src/test/resources/mailetcontainer.xml
4506  4898698d server/container/guice/jpa-smtp/src/test/resources/mailetcontainer.xml
4543  4b3ae506 server/container/guice/jpa-guice/src/test/resources/mailetcontainer.xml
5363  2b193774 server/protocols/jmap-draft-integration-testing/memory-jmap-draft-integration-testing/src/test/resources/mailetcontainer.xml
5365  9a3c1faa server/protocols/jmap-draft-integration-testing/cassandra-jmap-draft-integration-testing/src/test/resources/mailetcontainer.xml
5365  9a3c1faa server/protocols/jmap-draft-integration-testing/rabbitmq-jmap-draft-integration-testing/src/test/resources/mailetcontainer.xml
5511  eeab8845 dockerfiles/run/guice/jpa/destination/conf/mailetcontainer.xml
5544  819ee4a5 dockerfiles/run/spring/destination/conf/mailetcontainer.xml
5709  ca2e8418 server/container/guice/cassandra-rabbitmq-guice/src/test/resources/mailetcontainer.xml
5833  23de4b5b dockerfiles/run/guice/cassandra-ldap/destination/conf/mailetcontainer.xml
5858  f6a91174 dockerfiles/packaging/guice/cassandra/package/etc/james/templates/mailetcontainer.xml
5863  7ad26bc2 server/container/guice/memory-guice/src/test/resources/mailetcontainer.xml
5926  f4a69f67 dockerfiles/run/guice/memory/destination/conf/mailetcontainer.xml
5938  1c46209b dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/mailetcontainer.xml
5938  1c46209b dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/mailetcontainer.xml
5938  1c46209b dockerfiles/run/guice/cassandra/destination/conf/mailetcontainer.xml
6043  7f97674b server/container/guice/cassandra-guice/src/test/resources/mailetcontainer.xml
6828  85519195 examples/custom-mailets/src/main/resources/mailetcontainer.xml
22941 1e55d2da server/app/src/main/resources/mailetcontainer.xml

Copy link
Member

@mbaechler mbaechler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside the migration concern, it's a good enhancement. Thanks for your contribution

* <pre>
* <code>
* &lt;mailet matcher=??? class=ICALToHeader&gt;
* &lt;mailet match=??? class=ICALToHeader&gt;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does that mean we break existing configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK the james code, only the attribute match is considered. It might have been called matcher before, but I don't know that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh ok, I didn't understood. That's ok then.

@Arsnael
Copy link
Contributor

Arsnael commented Nov 15, 2019

Thank you for your contribution !

By the way the files that are really used as a standard for products configuration are the ones with the root folder dockerfiles... Those should be better commented indeed. The other ones however are just intended for testing related modules

@chibenwa
Copy link
Contributor

Hi,

This pull request has just been merged.

One very last thing: I don't have write access to this repository, hence I can not close this issue. Wouldn't you mind doing it for me? It would avoid me annoying the Apache INFRA team with such simple concerns.

Thanks again,

Cheers,

Benoit

@jtconsol jtconsol closed this Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants