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.

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

WIP: Refactor to individual rules #4

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

WIP: Refactor to individual rules #4

wants to merge 7 commits into from

Conversation

imliam
Copy link
Owner

@imliam imliam commented Sep 12, 2020

Refactors the code to use a rule-based approach where every rule is its own class that can do two things to the current email address; normalize it, and build a  regex it.

This is currently just a first pass based on the discussion in #1


public function regex(string $email): string
{
[$username, $domain] = explode('@', $email, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 This is redundant: the other method also contains this line

Copy link
Owner Author

Choose a reason for hiding this comment

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

I don't see how it's redundant - it's a different function and other than this one line, the functions do different things. I see this one line to split up the localpart and domain as simple enough to be duplicated when it's needed.

src/Rules/DotsInUsername.php Outdated Show resolved Hide resolved
src/Rules/DotsInUsername.php Outdated Show resolved Hide resolved
return false;
}

return in_array('aspmx.l.google.com', $mxRecords);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return in_array('aspmx.l.google.com', $mxRecords);
return in_array('aspmx.l.google.com', $mxRecords) || in_array('ASPMX.L.GOOGLE.COM', $mxRecords);

Copy link
Contributor

@szepeviktor szepeviktor Sep 12, 2020

Choose a reason for hiding this comment

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

Actually all mail exchangers should be converted to lowercase then compared.
Try return array_udiff(['aspmx.l.google.com'], $mxRecords, 'strcasecmp') === [];
I could be wrong.

@szepeviktor
Copy link
Contributor

szepeviktor commented Sep 13, 2020

@imliam This is my first take on the matter: https://github.com/szepeviktor/unique-email-address
Includes a Gmail class for you.

return '^' . preg_quote($this->email) . '$'; // Must be an exact match
if (Util::isGmailAddress($this->email) || Util::isGsuiteAddress($this->email)) {
$this->rules[GmailDomain::class] = new GmailDomain();
// $this->rules[MixedCase::class] = new MixedCase();
Copy link
Contributor

@szepeviktor szepeviktor Sep 13, 2020

Choose a reason for hiding this comment

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

From: =?utf-8?b?U1rDiVBF?= Viktor <[email protected]>
To: [email protected]
Subject: case

Gmail is case-insensitive. Also domain names are case-insensitive.

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.

2 participants