Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Message#== - comparison strategy #1529

Open
sebbASF opened this issue Dec 9, 2022 · 3 comments · May be fixed by #1562
Open

Message#== - comparison strategy #1529

sebbASF opened this issue Dec 9, 2022 · 3 comments · May be fixed by #1562

Comments

@sebbASF
Copy link
Collaborator

sebbASF commented Dec 9, 2022

The == method has to generate dummy message-ids if none exist.

As it stands, if either message does not have an id, then both are set to the same fixed value.

This seems fine for the case where neither message has an id, but is it the best strategy when one already has an id and the other does not?

Similarly for Dates (see #1527)
@sebbASF
Copy link
Collaborator Author

sebbASF commented Dec 10, 2022

If both messages have ids, the == method invokes encoded without first creating a copy.

This changes the original instance, which does not seem right.

It would be better to always create a duplicate.
The duplicate could then have Date and Message-Id added if not present.

@sebbASF
Copy link
Collaborator Author

sebbASF commented Dec 10, 2022

There are currently two tests which check that a provided message-id compares equal with nil.
These would need to be changed:

mail/spec/mail/message_spec.rb

Line 1719 in bf02b67

it "should ignore the message id value if self has a nil message id" do

and

mail/spec/mail/message_spec.rb

Line 1730 in bf02b67

it "should ignore the message id value if other has a nil message id" do

@sebbASF
Copy link
Collaborator Author

sebbASF commented Dec 14, 2022

Allowing Message-Id to match against nil causes unexpected behaviour for the == operator

See #1540

At present, Message equality (==) is not transitive.

sebbASF added a commit that referenced this issue Jan 24, 2023
@sebbASF
The == operator should be transitive
924b4a3 
This fixes #1529
@sebbASF sebbASF linked a pull request Jan 24, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

The == operator should be transitive mikel/mail
1 participant
@sebbASF