diff --git a/spring-cloud-info/src/test/java/org/springframework/cloud/info/SpringCloudInfoTestData.java b/spring-cloud-info/src/test/java/org/springframework/cloud/info/SpringCloudInfoTestData.java index ece1d626..d157790d 100644 --- a/spring-cloud-info/src/test/java/org/springframework/cloud/info/SpringCloudInfoTestData.java +++ b/spring-cloud-info/src/test/java/org/springframework/cloud/info/SpringCloudInfoTestData.java @@ -24,7 +24,11 @@ import java.util.Map; /** * @author Ryan Baxter */ -public class SpringCloudInfoTestData { +public final class SpringCloudInfoTestData { + + private SpringCloudInfoTestData() { + throw new IllegalStateException("Can't instantiate utility class"); + } public static Map releaseVersions = new HashMap<>(); static { diff --git a/spring-cloud-release-tools-core/pom.xml b/spring-cloud-release-tools-core/pom.xml index d8a5a63d..40cddf35 100644 --- a/spring-cloud-release-tools-core/pom.xml +++ b/spring-cloud-release-tools-core/pom.xml @@ -131,6 +131,12 @@ + + org.glassfish + javax.json + 1.0.4 + compile + diff --git a/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/CachingGithub.java b/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/CachingGithub.java new file mode 100644 index 00000000..834c1804 --- /dev/null +++ b/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/CachingGithub.java @@ -0,0 +1,374 @@ +/* + * Copyright 2013-2019 the original author or authors. + * + * 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 + * + * https://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.springframework.cloud.release.internal.git; + +import java.io.IOException; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; + +import javax.json.JsonObject; + +import com.jcabi.github.Assignees; +import com.jcabi.github.Branches; +import com.jcabi.github.Collaborators; +import com.jcabi.github.Contents; +import com.jcabi.github.Coordinates; +import com.jcabi.github.DeployKeys; +import com.jcabi.github.Forks; +import com.jcabi.github.Gists; +import com.jcabi.github.Git; +import com.jcabi.github.Github; +import com.jcabi.github.Gitignores; +import com.jcabi.github.Hooks; +import com.jcabi.github.IssueEvents; +import com.jcabi.github.Issues; +import com.jcabi.github.Labels; +import com.jcabi.github.Language; +import com.jcabi.github.Limits; +import com.jcabi.github.Markdown; +import com.jcabi.github.Milestones; +import com.jcabi.github.Notifications; +import com.jcabi.github.Organizations; +import com.jcabi.github.Pulls; +import com.jcabi.github.Releases; +import com.jcabi.github.Repo; +import com.jcabi.github.RepoCommits; +import com.jcabi.github.Repos; +import com.jcabi.github.Search; +import com.jcabi.github.Stars; +import com.jcabi.github.Users; +import com.jcabi.http.Request; + +class CachingGithub implements Github { + + private final AtomicReference repos = new AtomicReference<>(); + + private final Github delegate; + + CachingGithub(Github delegate) { + this.delegate = delegate; + } + + @Override + public Request entry() { + return this.delegate.entry(); + } + + @Override + public Repos repos() { + Repos repos = this.repos.get(); + if (repos == null) { + this.repos.set(new CachingRepos(this.delegate.repos())); + } + return this.repos.get(); + } + + @Override + public Gists gists() { + return this.delegate.gists(); + } + + @Override + public Users users() { + return this.delegate.users(); + } + + @Override + public Organizations organizations() { + return this.delegate.organizations(); + } + + @Override + public Markdown markdown() { + return this.delegate.markdown(); + } + + @Override + public Limits limits() { + return this.delegate.limits(); + } + + @Override + public Search search() { + return this.delegate.search(); + } + + @Override + public Gitignores gitignores() throws IOException { + return this.delegate.gitignores(); + } + + @Override + public JsonObject meta() throws IOException { + return this.delegate.meta(); + } + + @Override + public JsonObject emojis() throws IOException { + return this.delegate.emojis(); + } + + @Override + public boolean equals(Object o) { + return this.delegate.equals(o); + } + + @Override + public int hashCode() { + return this.delegate.hashCode(); + } + +} + +class CachingRepos implements Repos { + + private static final Map CACHE = new ConcurrentHashMap<>(); + + private final Repos delegate; + + CachingRepos(Repos delegate) { + this.delegate = delegate; + } + + @Override + public Github github() { + return this.delegate.github(); + } + + @Override + public Repo create(RepoCreate repoCreate) throws IOException { + return this.delegate.create(repoCreate); + } + + @Override + public Repo get(Coordinates coordinates) { + return CACHE.computeIfAbsent(coordinates, + o -> new CachingRepo(this.delegate.get(coordinates))); + } + + @Override + public void remove(Coordinates coordinates) throws IOException { + this.delegate.remove(coordinates); + } + + @Override + public Iterable iterate(String s) { + return this.delegate.iterate(s); + } + + @Override + public boolean equals(Object o) { + return this.delegate.equals(o); + } + + @Override + public int hashCode() { + return this.delegate.hashCode(); + } + +} + +class CachingRepo implements Repo { + + private static final Map CACHE = new ConcurrentHashMap<>(); + + private final Repo delegate; + + CachingRepo(Repo delegate) { + this.delegate = delegate; + } + + @Override + public Github github() { + return this.delegate.github(); + } + + @Override + public Coordinates coordinates() { + return (Coordinates) CACHE.computeIfAbsent( + new RepoKey(this.delegate, "coordinates"), + s -> this.delegate.coordinates()); + } + + @Override + public Issues issues() { + return (Issues) CACHE.computeIfAbsent(new RepoKey(this.delegate, "issues"), + s -> this.delegate.issues()); + } + + @Override + public Milestones milestones() { + return (Milestones) CACHE.computeIfAbsent( + new RepoKey(this.delegate, "milestones"), + s -> this.delegate.milestones()); + } + + @Override + public Pulls pulls() { + return (Pulls) CACHE.computeIfAbsent(new RepoKey(this.delegate, "pulls"), + s -> this.delegate.pulls()); + } + + @Override + public Hooks hooks() { + return (Hooks) CACHE.computeIfAbsent(new RepoKey(this.delegate, "hooks"), + s -> this.delegate.hooks()); + } + + @Override + public IssueEvents issueEvents() { + return (IssueEvents) CACHE.computeIfAbsent( + new RepoKey(this.delegate, "issueEvents"), + s -> this.delegate.issueEvents()); + } + + @Override + public Labels labels() { + return (Labels) CACHE.computeIfAbsent(new RepoKey(this.delegate, "labels"), + s -> this.delegate.labels()); + } + + @Override + public Assignees assignees() { + return (Assignees) CACHE.computeIfAbsent(new RepoKey(this.delegate, "assignees"), + s -> this.delegate.assignees()); + } + + @Override + public Releases releases() { + return (Releases) CACHE.computeIfAbsent(new RepoKey(this.delegate, "releases"), + s -> this.delegate.releases()); + } + + @Override + public DeployKeys keys() { + return (DeployKeys) CACHE.computeIfAbsent(new RepoKey(this.delegate, "keys"), + s -> this.delegate.keys()); + } + + @Override + public Forks forks() { + return (Forks) CACHE.computeIfAbsent(new RepoKey(this.delegate, "forks"), + s -> this.delegate.forks()); + } + + @Override + public RepoCommits commits() { + return (RepoCommits) CACHE.computeIfAbsent( + new RepoKey(this.delegate, "repoCommits"), s -> this.delegate.commits()); + } + + @Override + public Branches branches() { + return (Branches) CACHE.computeIfAbsent(new RepoKey(this.delegate, "branches"), + s -> this.delegate.branches()); + } + + @Override + public Contents contents() { + return (Contents) CACHE.computeIfAbsent(new RepoKey(this.delegate, "contents"), + s -> this.delegate.contents()); + } + + @Override + public Collaborators collaborators() { + return (Collaborators) CACHE.computeIfAbsent( + new RepoKey(this.delegate, "collaborators"), + s -> this.delegate.collaborators()); + } + + @Override + public Git git() { + return (Git) CACHE.computeIfAbsent(new RepoKey(this.delegate, "git"), + s -> this.delegate.git()); + } + + @Override + public Stars stars() { + return (Stars) CACHE.computeIfAbsent(new RepoKey(this.delegate, "stars"), + s -> this.delegate.stars()); + } + + @Override + public Notifications notifications() { + return (Notifications) CACHE.computeIfAbsent( + new RepoKey(this.delegate, "notifications"), + s -> this.delegate.notifications()); + } + + @Override + public Iterable languages() throws IOException { + return this.delegate.languages(); + } + + @Override + public JsonObject json() throws IOException { + return (JsonObject) CACHE.computeIfAbsent(new RepoKey(this.delegate, "json"), + s -> { + try { + return this.delegate.json(); + } + catch (IOException ex) { + throw new IllegalStateException(ex); + } + }); + } + + @Override + public void patch(JsonObject jsonObject) throws IOException { + this.delegate.patch(jsonObject); + } + + @Override + public int compareTo(Repo o) { + return this.delegate.compareTo(o); + } + +} + +class RepoKey { + + final Repo repo; + + final String key; + + RepoKey(Repo repo, String key) { + this.repo = repo; + this.key = key; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + RepoKey repoKey = (RepoKey) o; + return Objects.equals(this.repo, repoKey.repo) + && Objects.equals(this.key, repoKey.key); + } + + @Override + public int hashCode() { + return Objects.hash(this.repo, this.key); + } + +} diff --git a/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubIssues.java b/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubIssues.java index cd95bb05..39d8ff6b 100644 --- a/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubIssues.java +++ b/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubIssues.java @@ -49,13 +49,12 @@ class GithubIssues { private final ReleaserProperties properties; GithubIssues(ReleaserProperties properties) { - this.github = new RtGithub(new RtGithub(properties.getGit().getOauthToken()) - .entry().through(RetryWire.class)); - this.properties = properties; + this(new RtGithub(new RtGithub(properties.getGit().getOauthToken()).entry() + .through(RetryWire.class)), properties); } GithubIssues(Github github, ReleaserProperties properties) { - this.github = github; + this.github = new CachingGithub(github); this.properties = properties; } diff --git a/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubMilestones.java b/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubMilestones.java index 41d13811..ac3918f8 100644 --- a/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubMilestones.java +++ b/spring-cloud-release-tools-core/src/main/java/org/springframework/cloud/release/internal/git/GithubMilestones.java @@ -51,13 +51,12 @@ class GithubMilestones { private final ReleaserProperties properties; GithubMilestones(ReleaserProperties properties) { - this.github = new RtGithub(new RtGithub(properties.getGit().getOauthToken()) - .entry().through(RetryWire.class)); - this.properties = properties; + this(new RtGithub(new RtGithub(properties.getGit().getOauthToken()).entry() + .through(RetryWire.class)), properties); } GithubMilestones(Github github, ReleaserProperties properties) { - this.github = github; + this.github = new CachingGithub(github); this.properties = properties; } diff --git a/spring-cloud-release-tools-core/src/test/java/org/springframework/cloud/release/internal/git/CachingGithubTests.java b/spring-cloud-release-tools-core/src/test/java/org/springframework/cloud/release/internal/git/CachingGithubTests.java new file mode 100644 index 00000000..ca20da9d --- /dev/null +++ b/spring-cloud-release-tools-core/src/test/java/org/springframework/cloud/release/internal/git/CachingGithubTests.java @@ -0,0 +1,219 @@ +/* + * Copyright 2013-2019 the original author or authors. + * + * 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 + * + * https://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.springframework.cloud.release.internal.git; + +import com.jcabi.github.Coordinates; +import com.jcabi.github.Github; +import com.jcabi.github.Issues; +import com.jcabi.github.Milestones; +import com.jcabi.github.Releases; +import com.jcabi.github.Repo; +import com.jcabi.github.Repos; +import org.junit.jupiter.api.Test; +import org.mockito.BDDMockito; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.verify; + +class CachingGithubTests { + + @Test + void should_call_repos_only_once_for_same_github() { + Github github = mock(Github.class); + Repos repos = mock(Repos.class); + given(github.repos()).willReturn(repos); + CachingGithub cachingGithub = new CachingGithub(github); + + cachingGithub.repos(); + cachingGithub.repos(); + cachingGithub.repos(); + + verify(github, only()).repos(); + } + + @Test + void should_not_cache_repos_calls_for_different_githubs() { + Github github1 = mock(Github.class); + Github github2 = mock(Github.class); + Github github3 = mock(Github.class); + + new CachingGithub(github1).repos(); + new CachingGithub(github2).repos(); + new CachingGithub(github3).repos(); + + verify(github1).repos(); + verify(github2).repos(); + verify(github3).repos(); + } + + @Test + void should_get_repo_only_once_for_same_coordinates() { + Repos repos = mock(Repos.class); + CachingRepos cachingRepos = new CachingRepos(repos); + Coordinates.Simple simple = new Coordinates.Simple("foo", "bar"); + + cachingRepos.get(simple); + cachingRepos.get(simple); + cachingRepos.get(simple); + + verify(repos, only()).get(simple); + } + + @Test + void should_not_cache_calls_for_different_coordinates() { + Repos repos = mock(Repos.class); + CachingRepos cachingRepos = new CachingRepos(repos); + Coordinates.Simple simple1 = new Coordinates.Simple("foo", "bar1"); + Coordinates.Simple simple2 = new Coordinates.Simple("foo", "bar2"); + Coordinates.Simple simple3 = new Coordinates.Simple("foo", "bar3"); + + cachingRepos.get(simple1); + cachingRepos.get(simple2); + cachingRepos.get(simple3); + + verify(repos).get(simple1); + verify(repos).get(simple2); + verify(repos).get(simple3); + } + + @Test + void should_coordinates_only_once_for_same_repo() { + Repo repo = mock(Repo.class); + given(repo.coordinates()).willReturn(new Coordinates.Simple("foo", "bar")); + CachingRepo cachingRepo = new CachingRepo(repo); + + cachingRepo.coordinates(); + cachingRepo.coordinates(); + cachingRepo.coordinates(); + + verify(repo, only()).coordinates(); + } + + @Test + void should_not_cache_calls_for_different_coordinates_for_repo() { + Repo repo1 = mock(Repo.class); + given(repo1.coordinates()).willReturn(new Coordinates.Simple("foo", "bar")); + Repo repo2 = mock(Repo.class); + given(repo2.coordinates()).willReturn(new Coordinates.Simple("foo", "bar")); + Repo repo3 = mock(Repo.class); + given(repo3.coordinates()).willReturn(new Coordinates.Simple("foo", "bar")); + + new CachingRepo(repo1).coordinates(); + new CachingRepo(repo2).coordinates(); + new CachingRepo(repo3).coordinates(); + + verify(repo1).coordinates(); + verify(repo2).coordinates(); + verify(repo3).coordinates(); + } + + @Test + void should_issues_only_once_for_same_repo() { + Repo repo = mock(Repo.class); + given(repo.issues()).willReturn(BDDMockito.mock(Issues.class)); + CachingRepo cachingRepo = new CachingRepo(repo); + + cachingRepo.issues(); + cachingRepo.issues(); + cachingRepo.issues(); + + verify(repo, only()).issues(); + } + + @Test + void should_not_cache_calls_for_different_issues_for_repo() { + Repo repo1 = mock(Repo.class); + given(repo1.issues()).willReturn(BDDMockito.mock(Issues.class)); + Repo repo2 = mock(Repo.class); + given(repo2.issues()).willReturn(BDDMockito.mock(Issues.class)); + Repo repo3 = mock(Repo.class); + given(repo3.issues()).willReturn(BDDMockito.mock(Issues.class)); + + new CachingRepo(repo1).issues(); + new CachingRepo(repo2).issues(); + new CachingRepo(repo3).issues(); + + verify(repo1).issues(); + verify(repo2).issues(); + verify(repo3).issues(); + } + + @Test + void should_call_milestones_only_once_for_same_repo() { + Repo repo = mock(Repo.class); + given(repo.milestones()).willReturn(BDDMockito.mock(Milestones.class)); + CachingRepo cachingRepo = new CachingRepo(repo); + + cachingRepo.milestones(); + cachingRepo.milestones(); + cachingRepo.milestones(); + + verify(repo, only()).milestones(); + } + + @Test + void should_not_cache_calls_for_different_milestones_for_repo() { + Repo repo1 = mock(Repo.class); + given(repo1.milestones()).willReturn(BDDMockito.mock(Milestones.class)); + Repo repo2 = mock(Repo.class); + given(repo2.milestones()).willReturn(BDDMockito.mock(Milestones.class)); + Repo repo3 = mock(Repo.class); + given(repo3.milestones()).willReturn(BDDMockito.mock(Milestones.class)); + + new CachingRepo(repo1).milestones(); + new CachingRepo(repo2).milestones(); + new CachingRepo(repo3).milestones(); + + verify(repo1).milestones(); + verify(repo2).milestones(); + verify(repo3).milestones(); + } + + @Test + void should_call_releases_only_once_for_same_repo() { + Repo repo = mock(Repo.class); + given(repo.releases()).willReturn(BDDMockito.mock(Releases.class)); + CachingRepo cachingRepo = new CachingRepo(repo); + + cachingRepo.releases(); + cachingRepo.releases(); + cachingRepo.releases(); + + verify(repo, only()).releases(); + } + + @Test + void should_not_cache_calls_for_different_releases_for_repo() { + Repo repo1 = mock(Repo.class); + given(repo1.releases()).willReturn(BDDMockito.mock(Releases.class)); + Repo repo2 = mock(Repo.class); + given(repo2.releases()).willReturn(BDDMockito.mock(Releases.class)); + Repo repo3 = mock(Repo.class); + given(repo3.releases()).willReturn(BDDMockito.mock(Releases.class)); + + new CachingRepo(repo1).releases(); + new CachingRepo(repo2).releases(); + new CachingRepo(repo3).releases(); + + verify(repo1).releases(); + verify(repo2).releases(); + verify(repo3).releases(); + } + +}