Skip to content

Commit

Permalink
Refactor MetadataService to Use ContentTransformer (#1069)
Browse files Browse the repository at this point in the history
Motivation:
Previously, the Central Dogma server in `MetadataService` would repeatedly attempt to commit when
- `ChangeConflictException` is raised.
- The revision for fetching the content differs from the repository head revision.
With the introduction of `ContentTransformer` in #1064, the server can now handle transformations in a single attempt, avoiding unnecessary retries.

Modifications:
- Updated `MetadataService` to utilize `ContentTransformer` for commit operations.

Result:
- Improved efficiency in `MetadataService` by eliminating repeated commit attempts.

To-do:
- Encapsulate `tokenRepo` and `metadataRepo` in `MetadataService` into its class respectively.
  • Loading branch information
minwoox authored Dec 10, 2024
1 parent 3a261ba commit 810d02f
Show file tree
Hide file tree
Showing 8 changed files with 351 additions and 217 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,33 @@

package com.linecorp.centraldogma.common;

import static java.util.Objects.requireNonNull;

import javax.annotation.Nullable;

/**
* A {@link CentralDogmaException} that is raised when attempted to push a commit without effective changes.
*/
public class RedundantChangeException extends CentralDogmaException {

private static final long serialVersionUID = 8739464985038079688L;

/**
* Creates a new instance.
*/
public RedundantChangeException() {}
@Nullable
private final Revision headRevision;

/**
* Creates a new instance.
*/
public RedundantChangeException(String message) {
super(message);
headRevision = null;
}

/**
* Creates a new instance.
*/
public RedundantChangeException(Throwable cause) {
super(cause);
}

/**
* Creates a new instance.
*/
public RedundantChangeException(String message, Throwable cause) {
super(message, cause);
public RedundantChangeException(Revision headRevision, String message) {
super(message);
this.headRevision = requireNonNull(headRevision, "headRevision");
}

/**
Expand All @@ -57,13 +53,14 @@ public RedundantChangeException(String message, Throwable cause) {
*/
public RedundantChangeException(String message, boolean writableStackTrace) {
super(message, writableStackTrace);
headRevision = null;
}

/**
* Creates a new instance.
* Returns the head revision of the repository when this exception was raised.
*/
protected RedundantChangeException(String message, Throwable cause, boolean enableSuppression,
boolean writableStackTrace) {
super(message, cause, enableSuppression, writableStackTrace);
@Nullable
public Revision headRevision() {
return headRevision;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.linecorp.centraldogma.common.CentralDogmaException;
import com.linecorp.centraldogma.common.Revision;
import com.linecorp.centraldogma.internal.Jackson;
import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException;
import com.linecorp.centraldogma.server.storage.StorageException;

abstract class AbstractChangesApplier {
Expand All @@ -59,7 +60,7 @@ int apply(Repository jGitRepository, @Nullable Revision baseRevision,
}

return doApply(dirCache, reader, inserter);
} catch (CentralDogmaException | IllegalArgumentException e) {
} catch (CentralDogmaException | TokenNotFoundException | IllegalArgumentException e) {
throw e;
} catch (Exception e) {
throw new StorageException("failed to apply changes on revision: " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,12 @@ RevisionAndEntries commit(@Nullable Revision prevRevision, Revision nextRevision
}

if (!allowEmptyCommit && isEmpty) {
// prevRevision is not null when allowEmptyCommit is false.
assert prevRevision != null;
throw new RedundantChangeException(
prevRevision,
"changes did not change anything in " + gitRepository.parent().name() + '/' +
gitRepository.name() + " at revision " +
(prevRevision != null ? prevRevision.major() : 0) + ": " + changes);
gitRepository.name() + " at revision " + prevRevision.major() + ": " + changes);
}

// flush the current index to repository and get the result tree object id.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.google.common.base.MoreObjects;

import com.linecorp.centraldogma.common.CentralDogmaException;
import com.linecorp.centraldogma.common.ChangeConflictException;
import com.linecorp.centraldogma.common.EntryType;
import com.linecorp.centraldogma.internal.Jackson;
import com.linecorp.centraldogma.server.command.ContentTransformer;
import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException;

final class TransformingChangesApplier extends AbstractChangesApplier {

Expand Down Expand Up @@ -61,6 +63,8 @@ int doApply(DirCache dirCache, ObjectReader reader, ObjectInserter inserter) thr
applyPathEdit(dirCache, new InsertJson(changePath, inserter, newJsonNode));
return 1;
}
} catch (CentralDogmaException | TokenNotFoundException | IllegalArgumentException e) {
throw e;
} catch (Exception e) {
throw new ChangeConflictException("failed to transform the content: " + oldJsonNode +
" transformer: " + transformer, e);
Expand Down
Loading

0 comments on commit 810d02f

Please sign in to comment.