chore: git: write only changed files to the FS (#40623)
## Description This PR ensures that the file contents have changed before we start writing un-commited changed to the FS. We are seeing around 30% improvement in the latency of both git status api and git auto commit api with this change Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14924855165> > Commit: dc53aa6b15c0137d16c1e41d8ee5d2adf392bfad > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14924855165&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Fri, 09 May 2025 09:37:32 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved detection of file changes between the database and filesystem for more accurate updates in Git repositories. - **Bug Fixes** - Enhanced logic to ensure obsolete files are deleted and only changed resources are updated in the repository. - **Chores** - Updated variable names for improved clarity in internal processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
d036064beb
commit
8895534538
|
|
@ -251,7 +251,7 @@ public class FileUtilsCEImpl implements FileInterface {
|
|||
|
||||
@Override
|
||||
public Mono<Path> saveArtifactToGitRepo(
|
||||
Path baseRepoSuffix, GitResourceMap gitResourceMap, String branchName, boolean keepWorkingDirChanges)
|
||||
Path baseRepoSuffix, GitResourceMap gitResourceMapFromDB, String branchName, boolean keepWorkingDirChanges)
|
||||
throws GitAPIException, IOException {
|
||||
|
||||
// Repo path will be:
|
||||
|
|
@ -261,14 +261,18 @@ public class FileUtilsCEImpl implements FileInterface {
|
|||
.resetToLastCommit(baseRepoSuffix, branchName, keepWorkingDirChanges)
|
||||
.flatMap(isSwitched -> {
|
||||
Path baseRepo = Paths.get(gitServiceConfig.getGitRootPath()).resolve(baseRepoSuffix);
|
||||
Mono<GitResourceMap> gitResourceMapFromFSMono = constructGitResourceMapFromGitRepo(
|
||||
baseRepoSuffix, branchName)
|
||||
.name("constructGitResourceMapFromGitRepo");
|
||||
|
||||
try {
|
||||
updateEntitiesInRepo(gitResourceMap, baseRepo);
|
||||
} catch (IOException e) {
|
||||
return Mono.error(e);
|
||||
}
|
||||
|
||||
return Mono.just(baseRepo);
|
||||
return gitResourceMapFromFSMono.flatMap(gitResourceMapFromFS -> {
|
||||
try {
|
||||
updateEntitiesInRepo(gitResourceMapFromDB, baseRepo, gitResourceMapFromFS);
|
||||
} catch (IOException e) {
|
||||
return Mono.error(e);
|
||||
}
|
||||
return Mono.just(baseRepo);
|
||||
});
|
||||
})
|
||||
.subscribeOn(scheduler);
|
||||
}
|
||||
|
|
@ -327,19 +331,28 @@ public class FileUtilsCEImpl implements FileInterface {
|
|||
}
|
||||
}
|
||||
|
||||
protected Set<String> updateEntitiesInRepo(GitResourceMap gitResourceMap, Path baseRepo) throws IOException {
|
||||
ModifiedResources modifiedResources = gitResourceMap.getModifiedResources();
|
||||
Map<GitResourceIdentity, Object> resourceMap = gitResourceMap.getGitResourceMap();
|
||||
protected Set<String> updateEntitiesInRepo(
|
||||
GitResourceMap gitResourceMapFromDB, Path baseRepo, GitResourceMap gitResourceMapFromFS)
|
||||
throws IOException {
|
||||
Map<GitResourceIdentity, Object> resourceMapFromDB = gitResourceMapFromDB.getGitResourceMap();
|
||||
|
||||
Set<String> filesInRepo = getExistingFilesInRepo(baseRepo);
|
||||
|
||||
Set<String> updatedFilesToBeSerialized = resourceMap.keySet().parallelStream()
|
||||
Set<String> filesInRepo = new HashSet<>();
|
||||
Set<String> updatedFiles = new HashSet<>();
|
||||
Set<String> filesInDB = resourceMapFromDB.keySet().parallelStream()
|
||||
.map(gitResourceIdentity -> gitResourceIdentity.getFilePath())
|
||||
.collect(Collectors.toSet());
|
||||
|
||||
Map<String, Object> filesInFS = gitResourceMapFromFS.getGitResourceMap().entrySet().parallelStream()
|
||||
.map(entry -> {
|
||||
filesInRepo.add(entry.getKey().getFilePath());
|
||||
return entry;
|
||||
})
|
||||
.collect(Collectors.toMap(entry -> entry.getKey().getFilePath(), entry -> entry.getValue()));
|
||||
|
||||
// Remove all files that need to be serialized from the existing files list, as well as the README file
|
||||
// What we are left with are all the files to be deleted
|
||||
filesInRepo.removeAll(updatedFilesToBeSerialized);
|
||||
|
||||
filesInRepo.removeAll(filesInDB);
|
||||
filesInRepo.remove(README_FILE_NAME);
|
||||
|
||||
// Delete all the files because they are no longer needed
|
||||
|
|
@ -357,20 +370,19 @@ public class FileUtilsCEImpl implements FileInterface {
|
|||
|
||||
// Now go through the resource map and based on resource type, check if the resource is modified before
|
||||
// serialization
|
||||
// Or simply choose the mechanism for serialization
|
||||
Map<GitResourceType, GitResourceType> modifiedResourcesTypes = getModifiedResourcesTypes();
|
||||
return resourceMap.entrySet().parallelStream()
|
||||
resourceMapFromDB.entrySet().parallelStream()
|
||||
.map(entry -> {
|
||||
GitResourceIdentity key = entry.getKey();
|
||||
boolean resourceUpdated = true;
|
||||
if (modifiedResourcesTypes.containsKey(key.getResourceType()) && modifiedResources != null) {
|
||||
GitResourceType comparisonType = modifiedResourcesTypes.get(key.getResourceType());
|
||||
|
||||
try {
|
||||
resourceUpdated =
|
||||
modifiedResources.isResourceUpdatedNew(comparisonType, key.getResourceIdentifier());
|
||||
fileOperations.hasFileChanged(entry.getValue(), filesInFS.get(key.getFilePath()));
|
||||
} catch (IOException e) {
|
||||
log.error("Error while checking if file has changed", e);
|
||||
}
|
||||
|
||||
if (resourceUpdated) {
|
||||
log.info("Resource updated: {}", key.getFilePath());
|
||||
String filePath = key.getFilePath();
|
||||
saveResourceCommon(entry.getValue(), baseRepo.resolve(filePath));
|
||||
|
||||
|
|
@ -380,6 +392,7 @@ public class FileUtilsCEImpl implements FileInterface {
|
|||
})
|
||||
.filter(Objects::nonNull)
|
||||
.collect(Collectors.toSet());
|
||||
return updatedFiles;
|
||||
}
|
||||
|
||||
protected Set<String> updateEntitiesInRepo(ApplicationGitReference applicationGitReference, Path baseRepo) {
|
||||
|
|
|
|||
|
|
@ -113,6 +113,28 @@ public class FileOperationsCEv2Impl implements FileOperationsCE {
|
|||
}
|
||||
}
|
||||
|
||||
public boolean hasFileChanged(Object sourceEntity, Object fsSourceEntity) throws IOException {
|
||||
if (fsSourceEntity == null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
boolean hasChanged = true;
|
||||
if (sourceEntity instanceof String s) {
|
||||
hasChanged = !s.equals(fsSourceEntity.toString());
|
||||
} else if (sourceEntity instanceof JSONObject json) {
|
||||
JsonNode sourceNode = objectMapper.readTree(json.toString());
|
||||
String contentToWrite = objectWriter.writeValueAsString(sourceNode);
|
||||
String fsContent = objectWriter.writeValueAsString(fsSourceEntity);
|
||||
hasChanged = !contentToWrite.equals(fsContent);
|
||||
} else {
|
||||
String contentToWrite = objectWriter.writeValueAsString(sourceEntity);
|
||||
String fsContent = objectWriter.writeValueAsString(fsSourceEntity);
|
||||
hasChanged = !contentToWrite.equals(fsContent);
|
||||
}
|
||||
|
||||
return hasChanged;
|
||||
}
|
||||
|
||||
/**
|
||||
* This method will be used to read and dehydrate the json file present from the local git repo
|
||||
*
|
||||
|
|
|
|||
|
|
@ -18,6 +18,8 @@ public interface FileOperationsCE {
|
|||
|
||||
boolean writeToFile(Object sourceEntity, Path path) throws IOException;
|
||||
|
||||
boolean hasFileChanged(Object sourceEntity, Object fsSourceEntity) throws IOException;
|
||||
|
||||
void scanAndDeleteFileForDeletedResources(Set<String> validResources, Path resourceDirectory);
|
||||
|
||||
void scanAndDeleteDirectoryForDeletedResources(Set<String> validResources, Path resourceDirectory);
|
||||
|
|
|
|||
|
|
@ -178,7 +178,7 @@ public class CommonGitFileUtilsCE {
|
|||
Path baseRepoSuffix, ArtifactExchangeJson artifactExchangeJson, String branchName) {
|
||||
|
||||
// this should come from the specific files
|
||||
GitResourceMap gitResourceMap = createGitResourceMap(artifactExchangeJson);
|
||||
GitResourceMap gitResourceMapFromDB = createGitResourceMap(artifactExchangeJson);
|
||||
Mono<Boolean> keepWorkingDirChangesMono =
|
||||
featureFlagService.check(FeatureFlagEnum.release_git_reset_optimization_enabled);
|
||||
|
||||
|
|
@ -186,7 +186,7 @@ public class CommonGitFileUtilsCE {
|
|||
return keepWorkingDirChangesMono.flatMap(keepWorkingDirChanges -> {
|
||||
try {
|
||||
return fileUtils
|
||||
.saveArtifactToGitRepo(baseRepoSuffix, gitResourceMap, branchName, keepWorkingDirChanges)
|
||||
.saveArtifactToGitRepo(baseRepoSuffix, gitResourceMapFromDB, branchName, keepWorkingDirChanges)
|
||||
.subscribeOn(Schedulers.boundedElastic());
|
||||
} catch (IOException | GitAPIException exception) {
|
||||
return Mono.error(exception);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user