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

Add Author as a field in Repository #682

Merged
merged 3 commits into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@
import java.util.Map;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.Executor;

import com.google.common.base.Throwables;

import com.linecorp.armeria.common.CommonPools;
import com.linecorp.armeria.common.RequestContext;
import com.linecorp.armeria.common.util.Exceptions;
Expand All @@ -48,7 +45,6 @@
import com.linecorp.centraldogma.common.RevisionRange;
import com.linecorp.centraldogma.server.command.CommitResult;
import com.linecorp.centraldogma.server.internal.storage.repository.RepositoryCache;
import com.linecorp.centraldogma.server.storage.StorageException;
import com.linecorp.centraldogma.server.storage.project.Project;
import com.linecorp.centraldogma.server.storage.repository.FindOption;
import com.linecorp.centraldogma.server.storage.repository.Repository;
Expand All @@ -60,30 +56,20 @@ final class CachingRepository implements Repository {

private final Repository repo;
private final RepositoryCache cache;
private final Commit firstCommit;

CachingRepository(Repository repo, RepositoryCache cache) {
this.repo = requireNonNull(repo, "repo");
this.cache = requireNonNull(cache, "cache");

try {
final List<Commit> history = repo.history(Revision.INIT, Revision.INIT, ALL_PATH, 1).join();
firstCommit = history.get(0);
} catch (CompletionException e) {
final Throwable cause = Exceptions.peel(e);
Throwables.throwIfUnchecked(cause);
throw new StorageException("failed to retrieve the initial commit", cause);
}
}

@Override
public long creationTimeMillis() {
return firstCommit.when();
return repo.creationTimeMillis();
}

@Override
public Author author() {
return firstCommit.author();
return repo.author();
}

@Override
Expand Down Expand Up @@ -325,7 +311,6 @@ public CompletableFuture<CommitResult> commit(Revision baseRevision, long commit
public String toString() {
return toStringHelper(this)
.add("repo", repo)
.add("firstCommit", firstCommit)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,10 @@ class GitRepository implements Repository {
private final ReadWriteLock rwLock = new ReentrantReadWriteLock();
private final Project parent;
private final Executor repositoryWorker;
private final long creationTimeMillis;
private final Author author;
@VisibleForTesting
@Nullable
final RepositoryCache cache;
private final String name;
private final org.eclipse.jgit.lib.Repository jGitRepository;
Expand Down Expand Up @@ -211,10 +214,10 @@ class GitRepository implements Repository {
this.parent = requireNonNull(parent, "parent");
name = requireNonNull(repoDir, "repoDir").getName();
this.repositoryWorker = requireNonNull(repositoryWorker, "repositoryWorker");
this.creationTimeMillis = creationTimeMillis;
this.author = requireNonNull(author, "author");
this.cache = cache;

requireNonNull(author, "author");

final RepositoryBuilder repositoryBuilder = new RepositoryBuilder().setGitDir(repoDir).setBare();
boolean success = false;
try {
Expand Down Expand Up @@ -315,6 +318,9 @@ class GitRepository implements Repository {
commitIdDatabase.rebuild(jGitRepository);
assert headRevision.equals(commitIdDatabase.headRevision());
}
final Commit initialCommit = blockingHistory(Revision.INIT, Revision.INIT, ALL_PATH, 1).get(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also update CachingRepository to delegate author and creationTimeMillis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed. 😉

creationTimeMillis = initialCommit.when();
author = initialCommit.author();
success = true;
} finally {
if (!success) {
Expand Down Expand Up @@ -393,6 +399,16 @@ public String name() {
return name;
}

@Override
public long creationTimeMillis() {
return creationTimeMillis;
}

@Override
public Author author() {
return author;
}

@Override
public Revision normalizeNow(Revision revision) {
return normalizeNow(revision, cachedHeadRevision().major());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;

import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.spotify.futures.CompletableFutures;
Expand Down Expand Up @@ -83,30 +81,12 @@ public interface Repository {
/**
* Returns the creation time of this {@link Repository}.
*/
default long creationTimeMillis() {
try {
final List<Commit> history = history(Revision.INIT, Revision.INIT, ALL_PATH, 1).join();
return history.get(0).when();
} catch (CompletionException e) {
final Throwable cause = Throwables.getRootCause(e);
Throwables.throwIfUnchecked(cause);
throw new StorageException("failed to retrieve the initial commit", cause);
}
}
long creationTimeMillis();

/**
* Returns the author who created this {@link Repository}.
*/
default Author author() {
try {
final List<Commit> history = history(Revision.INIT, Revision.INIT, ALL_PATH, 1).join();
return history.get(0).author();
} catch (CompletionException e) {
final Throwable cause = Throwables.getRootCause(e);
Throwables.throwIfUnchecked(cause);
throw new StorageException("failed to retrieve the initial commit", cause);
}
}
Author author();

/**
* Returns the {@link CompletableFuture} whose value is the absolute {@link Revision} of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,11 @@ void rebuildingBadDatabase() throws Exception {
final File commitIdDatabaseFile = new File(repoDir, "commit_ids.dat");

// Create a repository which contains some commits.
GitRepository repo = new GitRepository(mock(Project.class), repoDir, commonPool(), 0, Author.SYSTEM);
GitRepository repo = new GitRepository(mock(Project.class), repoDir, commonPool(), 1000, Author.SYSTEM);
Revision headRevision = null;
try {
for (int i = 1; i <= numCommits; i++) {
headRevision = repo.commit(new Revision(i), 0, Author.SYSTEM, "",
headRevision = repo.commit(new Revision(i), 0, Author.DEFAULT, "",
Change.ofTextUpsert("/" + i + ".txt", "")).join().revision();
}
} finally {
Expand All @@ -167,6 +167,8 @@ void rebuildingBadDatabase() throws Exception {
repo.internalClose();
}

assertThat(repo.creationTimeMillis()).isEqualTo(1000);
assertThat(repo.author()).isEqualTo(Author.SYSTEM);
assertThat(Files.size(commitIdDatabaseFile.toPath())).isEqualTo((numCommits + 1) * 24L);
}

Expand Down