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

Make Owner addressable via RFC 2822 standard #2411

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions allure-generator/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ dependencies {
implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml")
implementation("com.fasterxml.jackson.module:jackson-module-jaxb-annotations")
implementation("commons-io:commons-io")
implementation("commons-validator:commons-validator:1.7")
implementation("io.qameta.allure:allure-model")
implementation("javax.xml.bind:jaxb-api")
implementation("org.allurefw:allure1-model")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright 2016-2024 Qameta Software Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.qameta.allure.owner;

import lombok.Getter;

@Getter
public class OwnerAddress {
private final String displayName;
private final String url;

public OwnerAddress(final String displayName, final String url) {
this.displayName = displayName;
this.url = url;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright 2016-2024 Qameta Software Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.qameta.allure.owner;

import org.apache.commons.validator.routines.EmailValidator;
import org.apache.commons.validator.routines.UrlValidator;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

public final class OwnerAddressParser {
private static final Pattern RFC2822_ADDRESS = Pattern.compile("^([^<>]+)\\s+<\\s*(\\S*)\\s*>$");

private OwnerAddressParser() {
}

@SuppressWarnings("ReturnCount")
public static OwnerAddress parseAddress(final String maybeAddress) {
if (maybeAddress == null || maybeAddress.isEmpty()) {
return null;
}

// Prevent performance degradation for plain text
if (!isLikelyAddress(maybeAddress)) {
Copy link
Contributor Author

@noomorph noomorph May 9, 2024

Choose a reason for hiding this comment

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

This thing is important to have for plain text. On 10,000 "John Doe {{counter}}" addresses, it eats them in 20ms, just like without parsing (16-18ms).

Without this optimization, it takes up to 200ms on my machine to parse 10,000 addresses.

I measured the performance between my previous simplistic regexp implementation and the actual implementation with Apache validators, and the impact was negligible — ±20-30ms (sometimes faster, sometimes slower), so I hope this is totally okay for you to accept this PR with industry-proven validators for emails and URLs instead of improvised ones by me.

return new OwnerAddress(maybeAddress, null);
}

String displayName = maybeAddress;
String urlOrEmail = maybeAddress;

final Matcher matcher = RFC2822_ADDRESS.matcher(maybeAddress);
if (matcher.matches()) {
displayName = matcher.group(1);
urlOrEmail = matcher.group(2);
}

// e.g.: John Doe <>
if (urlOrEmail.isEmpty()) {
return new OwnerAddress(displayName, null);
}

// e.g.: John Doe <https://example.com>
if (UrlValidator.getInstance().isValid(urlOrEmail)) {
return new OwnerAddress(displayName, urlOrEmail);
}

// e.g.: John Doe <[email protected]>
if (EmailValidator.getInstance().isValid(urlOrEmail)) {
return new OwnerAddress(displayName, "mailto:" + urlOrEmail);
}

// Non-compliant addresses are treated as plain text
return new OwnerAddress(maybeAddress, null);
}

/**
* Checks if the given string is likely to be a plain text (not an email or URL).
* Regular expressions are slow, therefore we just check for common characters.
*/
private static boolean isLikelyAddress(final String input) {
return input.contains("@") || input.contains(":") || input.contains("<");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public void aggregate(final Configuration configuration,

private void setOwner(final TestResult result) {
result.findOneLabel(LabelName.OWNER)
.ifPresent(owner -> result.addExtraBlock(OWNER_BLOCK_NAME, owner));
.map(OwnerAddressParser::parseAddress)
.ifPresent(ownerAddress ->
result.addExtraBlock(OWNER_BLOCK_NAME, ownerAddress)
);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{{#if owner}}
<h3 class="pane__section-title">{{t 'testResult.owner.name'}}</h3>
<div>{{owner}}</div>
{{/if}}
{{t 'testResult.owner.name'}}:
{{#if owner.url}}
<a href="{{owner.url}}">{{owner.displayName}}</a>
{{else}}
{{owner.displayName}}
{{/if}}
{{/if}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Copyright 2016-2024 Qameta Software Inc
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.qameta.allure.owner;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class OwnerAddressParserTest {
@Test
void shouldReturnNullForNullInput() {
assertNull(OwnerAddressParser.parseAddress(null));
}

@Test
void shouldReturnNullForEmptyInput() {
assertNull(OwnerAddressParser.parseAddress(""));
}

Copy link

Choose a reason for hiding this comment

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

Tests look good.
For better test coverage it would be good to add a test for invalid URL and invalid email.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@A1exKH, I added

    implementation("commons-validator:commons-validator:1.7")

to Allure Generator build.kts to make it more resilient when it comes to validating URLs and emails. The remaining regexp for angle brackets parsing has been made a bit stricter too.

Please review the updated tests and implementation again. 🙏

@Test
void shouldParseRFC2822FormattedStringWithEmail() {
String input = "John Doe < [email protected] >";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Empty spaces in the angle brackets are considered normal thing by the standard.

OwnerAddress expected = new OwnerAddress("John Doe", "mailto:[email protected]");
assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(input).getDisplayName());
assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(input).getUrl());
}

@Test
void shouldParseRFC2822FormattedStringWithURL() {
String input = "John Doe <https://github.com/@john.doe>";
OwnerAddress expected = new OwnerAddress("John Doe", "https://github.com/@john.doe");
assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(input).getDisplayName());
assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(input).getUrl());
}

@Test
void shouldReturnOnlyDisplayNameForEmptyRFC822Address() {
String emptyAddress = "John Doe <>";
OwnerAddress actual = OwnerAddressParser.parseAddress(emptyAddress);
assertEquals("John Doe", actual.getDisplayName());
assertNull(actual.getUrl());
}

@Test
void shouldReturnDisplayNameForPlainTextInput() {
String displayName = "John Doe";
OwnerAddress expected = new OwnerAddress(displayName, null);
assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(displayName).getDisplayName());
assertNull(OwnerAddressParser.parseAddress(displayName).getUrl());
}

@Test
void shouldReturnDisplayNameAndUrlForEmailAddress() {
String email = "[email protected]";
OwnerAddress expected = new OwnerAddress(email, "mailto:" + email);
assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(email).getDisplayName());
assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(email).getUrl());
}

@Test
void shouldReturnDisplayNameAndUrlForValidURL() {
String validUrl = "https://github.com/john.doe";
OwnerAddress expected = new OwnerAddress(validUrl, validUrl);
assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(validUrl).getDisplayName());
assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(validUrl).getUrl());
}

@Test
void shouldReturnOnlyDisplayNameForInvalidURL() {
String invalidUrl = "htp:/www.example.com/page";
OwnerAddress actual = OwnerAddressParser.parseAddress(invalidUrl);
assertEquals(invalidUrl, actual.getDisplayName());
assertNull(actual.getUrl());
}

@Test
void shouldReturnOnlyDisplayNameForInvalidEmail() {
String invalidEmail = "[email protected]";
OwnerAddress actual = OwnerAddressParser.parseAddress(invalidEmail);
assertEquals(invalidEmail, actual.getDisplayName());
assertNull(actual.getUrl());
}

@Test
void shouldReturnInvalidRFC822AddressUnchanged() {
String invalidAddress = "John Doe <john@@doe>";
OwnerAddress actual = OwnerAddressParser.parseAddress(invalidAddress);
assertEquals(invalidAddress, actual.getDisplayName());
assertNull(actual.getUrl());
}
}
Loading