This is an automated email from the ASF dual-hosted git repository. chesnay pushed a commit to branch release-1.16 in repository https://gitbox.apache.org/repos/asf/flink.git
commit a76d8b8b5e6d39f08a0eb4f9e0d809d04f160645 Author: Chesnay Schepler <ches...@apache.org> AuthorDate: Wed Oct 5 15:41:41 2022 +0200 [FLINK-29510][tests] Test NOTICE file parsing --- .../tools/ci/licensecheck/NoticeFileChecker.java | 34 +++------ .../tools/ci/utils/notice/NoticeContents.java | 42 +++++++++++ .../flink/tools/ci/utils/notice/NoticeParser.java | 82 +++++++++++++++++++++ .../tools/ci/utils/notice/NoticeParserTest.java | 85 ++++++++++++++++++++++ 4 files changed, 221 insertions(+), 22 deletions(-) diff --git a/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java index 8ba7f95c449..e51da1d1d33 100644 --- a/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java +++ b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java @@ -19,6 +19,8 @@ package org.apache.flink.tools.ci.licensecheck; import org.apache.flink.tools.ci.utils.dependency.DependencyParser; +import org.apache.flink.tools.ci.utils.notice.NoticeContents; +import org.apache.flink.tools.ci.utils.notice.NoticeParser; import org.apache.flink.tools.ci.utils.shared.Dependency; import com.google.common.collect.ArrayListMultimap; @@ -175,44 +177,32 @@ public class NoticeFileChecker { String moduleName = getModuleFromNoticeFile(noticeFile); // 1st line contains module name - List<String> noticeContents = Files.readAllLines(noticeFile); + NoticeContents noticeContents = NoticeParser.parseNoticeFile(noticeFile).orElse(null); final Map<Severity, List<String>> problemsBySeverity = new HashMap<>(); - if (noticeContents.isEmpty()) { + if (noticeContents == null) { addProblem(problemsBySeverity, Severity.CRITICAL, "The NOTICE file was empty."); } // first line must be the module name. - if (!noticeContents.get(0).equals(moduleName)) { + if (!noticeContents.getNoticeModuleName().equals(moduleName)) { addProblem( problemsBySeverity, Severity.TOLERATED, String.format( "First line does not start with module name. firstLine=%s", - noticeContents.get(0))); + noticeContents.getNoticeModuleName())); } // collect all declared dependencies from NOTICE file Set<Dependency> declaredDependencies = new HashSet<>(); - for (String line : noticeContents) { - Matcher noticeDependencyMatcher = NOTICE_DEPENDENCY_PATTERN.matcher(line); - if (noticeDependencyMatcher.find()) { - String groupId = noticeDependencyMatcher.group(1); - String artifactId = noticeDependencyMatcher.group(2); - String version = noticeDependencyMatcher.group(3); - if (groupId == null && artifactId == null && version == null) { // "bundles" case - groupId = noticeDependencyMatcher.group(5); - artifactId = noticeDependencyMatcher.group(6); - version = noticeDependencyMatcher.group(7); - } - Dependency toAdd = Dependency.create(groupId, artifactId, version); - if (!declaredDependencies.add(toAdd)) { - addProblem( - problemsBySeverity, - Severity.CRITICAL, - String.format("Dependency %s is declared twice.", toAdd)); - } + for (Dependency toAdd : noticeContents.getDeclaredDependencies()) { + if (!declaredDependencies.add(toAdd)) { + addProblem( + problemsBySeverity, + Severity.CRITICAL, + String.format("Dependency %s is declared twice.", toAdd)); } } // find all dependencies missing from NOTICE file diff --git a/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/utils/notice/NoticeContents.java b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/utils/notice/NoticeContents.java new file mode 100644 index 00000000000..56162daed8c --- /dev/null +++ b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/utils/notice/NoticeContents.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.flink.tools.ci.utils.notice; + +import org.apache.flink.tools.ci.utils.shared.Dependency; + +import java.util.Collection; + +/** Represents the parsed contents of a NOTICE file. */ +public class NoticeContents { + private final String noticeModuleName; + private final Collection<Dependency> declaredDependencies; + + public NoticeContents(String noticeModuleName, Collection<Dependency> declaredDependencies) { + this.noticeModuleName = noticeModuleName; + this.declaredDependencies = declaredDependencies; + } + + public String getNoticeModuleName() { + return noticeModuleName; + } + + public Collection<Dependency> getDeclaredDependencies() { + return declaredDependencies; + } +} diff --git a/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/utils/notice/NoticeParser.java b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/utils/notice/NoticeParser.java new file mode 100644 index 00000000000..fcb18acb507 --- /dev/null +++ b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/utils/notice/NoticeParser.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.flink.tools.ci.utils.notice; + +import org.apache.flink.annotation.VisibleForTesting; +import org.apache.flink.tools.ci.utils.shared.Dependency; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** Parsing utils for NOTICE files. */ +public class NoticeParser { + + // "- org.apache.htrace:htrace-core:3.1.0-incubating" + private static final Pattern NOTICE_DEPENDENCY_PATTERN = + Pattern.compile("- (?<groupId>[^ :]+):(?<artifactId>[^:]+):(?<version>[^ ]+)($| )"); + // "This project bundles "net.jcip:jcip-annotations:1.0". + private static final Pattern NOTICE_BUNDLES_DEPENDENCY_PATTERN = + Pattern.compile( + ".*bundles \"(?<groupId>[^:]+):(?<artifactId>[^:]+):(?<version>[^\"]+)\".*"); + + public static Optional<NoticeContents> parseNoticeFile(Path noticeFile) throws IOException { + // 1st line contains module name + final List<String> noticeContents = Files.readAllLines(noticeFile); + + return parseNoticeFile(noticeContents); + } + + @VisibleForTesting + static Optional<NoticeContents> parseNoticeFile(List<String> noticeContents) { + if (noticeContents.isEmpty()) { + return Optional.empty(); + } + + final String noticeModuleName = noticeContents.get(0); + + Collection<Dependency> declaredDependencies = new ArrayList<>(); + for (String line : noticeContents) { + Optional<Dependency> dependency = tryParsing(NOTICE_DEPENDENCY_PATTERN, line); + if (!dependency.isPresent()) { + dependency = tryParsing(NOTICE_BUNDLES_DEPENDENCY_PATTERN, line); + } + dependency.ifPresent(declaredDependencies::add); + } + + return Optional.of(new NoticeContents(noticeModuleName, declaredDependencies)); + } + + private static Optional<Dependency> tryParsing(Pattern pattern, String line) { + Matcher matcher = pattern.matcher(line); + if (matcher.find()) { + String groupId = matcher.group("groupId"); + String artifactId = matcher.group("artifactId"); + String version = matcher.group("version"); + return Optional.of(Dependency.create(groupId, artifactId, version)); + } + return Optional.empty(); + } +} diff --git a/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/utils/notice/NoticeParserTest.java b/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/utils/notice/NoticeParserTest.java new file mode 100644 index 00000000000..87d39851e40 --- /dev/null +++ b/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/utils/notice/NoticeParserTest.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.flink.tools.ci.utils.notice; + +import org.apache.flink.tools.ci.utils.shared.Dependency; + +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class NoticeParserTest { + @Test + void testParseNoticeFileCommonPath() { + final String module = "some-module"; + final Dependency dependency1 = Dependency.create("groupId1", "artifactId1", "version1"); + final Dependency dependency2 = Dependency.create("groupId2", "artifactId2", "version2"); + final List<String> noticeContents = + Arrays.asList( + module, + "", + "Some text about the applicable license", + "- " + dependency1, + "- " + dependency2, + "", + "some epilogue"); + + assertThat(NoticeParser.parseNoticeFile(noticeContents)) + .hasValueSatisfying( + contents -> { + assertThat(contents.getNoticeModuleName()).isEqualTo(module); + assertThat(contents.getDeclaredDependencies()) + .containsExactlyInAnyOrder(dependency1, dependency2); + }); + } + + @Test + void testParseNoticeFileBundlesPath() { + final String module = "some-module"; + final Dependency dependency = Dependency.create("groupId", "artifactId", "version"); + final List<String> noticeContents = + Arrays.asList(module, "", "Something bundles \"" + dependency + "\""); + + assertThat(NoticeParser.parseNoticeFile(noticeContents)) + .hasValueSatisfying( + contents -> { + assertThat(contents.getNoticeModuleName()).isEqualTo(module); + assertThat(contents.getDeclaredDependencies()) + .containsExactlyInAnyOrder(dependency); + }); + } + + @Test + void testParseNoticeFileMalformedDependencyIgnored() { + final String module = "some-module"; + final Dependency dependency = Dependency.create("groupId", "artifactId", "version"); + final List<String> noticeContents = Arrays.asList(module, "- " + dependency, "- a:b"); + + assertThat(NoticeParser.parseNoticeFile(noticeContents)) + .hasValueSatisfying( + contents -> { + assertThat(contents.getNoticeModuleName()).isEqualTo(module); + assertThat(contents.getDeclaredDependencies()) + .containsExactlyInAnyOrder(dependency); + }); + } +}