Skip to content

chore: Switching to ref types and names for git #38433

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.

Sign up for GitHub

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

Merged
nidhi-nair merged 7 commits into from
Jan 2, 2025
Merged

chore: Switching to ref types and names for git #38433

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4f2d4e3
Simply introducing new vars
nidhi-nair Dec 31, 2024
1cb3a96
Allow case insensitive enums
nidhi-nair Dec 31, 2024
c74700c
Fixes
nidhi-nair Jan 1, 2025
b36cb91
Fix test mock
nidhi-nair Jan 1, 2025
fdc86c7
Merge branch 'release' into chore/ref-type
nidhi-nair Jan 1, 2025
f2690b4
Fixes
nidhi-nair Jan 1, 2025
5e2ad53
Fixes
nidhi-nair Jan 1, 2025
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
2 changes: 1 addition & 1 deletion app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ public Mono<String> createAndCheckoutReference(Path repoSuffix, GitRefDTO gitRef
refName,
repoSuffix);

if (RefType.TAG.equals(refType)) {
if (RefType.tag.equals(refType)) {
return createTag(git, gitRefDTO);
}

Expand Down
4 changes: 2 additions & 2 deletions ...ver/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/RefType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.appsmith.external.git.constants.ce;

public enum RefType {
BRANCH,
TAG
branch,
tag
}
2 changes: 1 addition & 1 deletion app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/Datasource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,5 +152,5 @@ public void nullifyStorageReplicaFields() {
this.setMessages(null);
}

public static class Fields extends BranchAwareDomain.Fields {}
public static class Fields extends RefAwareDomain.Fields {}
}
31 changes: 30 additions & 1 deletion ...th/external/models/BranchAwareDomain.java → ...smith/external/models/RefAwareDomain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.external.models;

import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.views.Views;
import com.fasterxml.jackson.annotation.JsonView;
import lombok.Getter;
Expand All @@ -9,14 +10,42 @@
@Setter
@Getter
@FieldNameConstants
public abstract class BranchAwareDomain extends GitSyncedDomain {
public abstract class RefAwareDomain extends GitSyncedDomain {

@JsonView(Views.Public.class)
String baseId;

@JsonView(Views.Internal.class)
String branchName;

@JsonView(Views.Internal.class)
RefType refType;

@JsonView(Views.Internal.class)
String refName;

public RefType getRefType() {
if (refType == null) {
if (this.getRefName() != null) {
return RefType.branch;
} else {
return null;
}
}
return refType;
}

public String getRefName() {
return refName == null ? branchName : refName;
}

public void setRefName(String refName) {
this.refName = refName;
if (this.getRefType() == RefType.branch) {
this.branchName = refName;
}
}

@JsonView(Views.Internal.class)
public String getBaseIdOrFallback() {
return baseId == null ? this.getId() : baseId;
Expand Down
7 changes: 6 additions & 1 deletion ...erver/appsmith-interfaces/src/main/java/com/appsmith/external/models/ce/ActionCE_DTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.appsmith.external.dtos.DslExecutableDTO;
import com.appsmith.external.dtos.LayoutExecutableUpdateDTO;
import com.appsmith.external.exceptions.ErrorDTO;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.helpers.Identifiable;
import com.appsmith.external.models.ActionConfiguration;
import com.appsmith.external.models.CreatorContextType;
Expand Down Expand Up @@ -172,7 +173,11 @@ public class ActionCE_DTO implements Identifiable, Executable {

@Transient
@JsonView({Views.Internal.class})
private String branchName;
private RefType refType;

@Transient
@JsonView({Views.Internal.class})
private String refName;

// TODO Abhijeet: Remove this method once we have migrated all the usages of policies to policyMap
/**
Expand Down
12 changes: 0 additions & 12 deletions ...r/src/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.appsmith.server.actioncollections.base;

import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.CreatorContextType;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.domains.ActionCollection;
import com.appsmith.server.domains.NewPage;
Expand All @@ -26,14 +25,9 @@ Flux<ActionCollection> findAllByApplicationIdAndViewMode(

Flux<ActionCollection> saveAll(List<ActionCollection> collections);

Mono<ActionCollection> findByBaseIdAndBranchName(String id, String branchName);

Flux<ActionCollectionDTO> getPopulatedActionCollectionsByViewMode(
MultiValueMap<String, String> params, Boolean viewMode);

Flux<ActionCollectionDTO> getPopulatedActionCollectionsByViewMode(
MultiValueMap<String, String> params, Boolean viewMode, String branchName);

Mono<ActionCollectionDTO> populateActionCollectionByViewMode(
ActionCollectionDTO actionCollectionDTO1, Boolean viewMode);

Expand Down Expand Up @@ -70,14 +64,8 @@ Flux<ActionCollectionDTO> getCollectionsByPageIdAndViewMode(

Mono<ActionCollection> archiveById(String id);

Mono<ActionCollection> findByBranchNameAndBaseCollectionId(
String branchName, String baseCollectionId, AclPermission permission);

Mono<List<ActionCollection>> archiveActionCollectionByApplicationId(String applicationId, AclPermission permission);

Flux<ActionCollection> findAllActionCollectionsByContextIdAndContextTypeAndViewMode(
String contextId, CreatorContextType contextType, AclPermission permission, boolean viewMode);

Mono<ActionCollectionDTO> validateAndSaveCollection(ActionCollection actionCollection);

Flux<ActionCollectionViewDTO> getActionCollectionsForViewMode(String branchedApplicationId);
Expand Down
49 changes: 2 additions & 47 deletions ...c/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.appsmith.server.actioncollections.base;

import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.CreatorContextType;
import com.appsmith.external.models.Policy;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.acl.PolicyGenerator;
Expand All @@ -26,7 +25,6 @@
import org.apache.commons.lang3.ObjectUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.domain.Sort;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
import reactor.core.observability.micrometer.Micrometer;
Expand Down Expand Up @@ -127,29 +125,13 @@ public Flux<ActionCollection> saveAll(List<ActionCollection> collections) {
return repository.saveAll(collections);
}

@Override
public Mono<ActionCollection> findByBaseIdAndBranchName(String id, String branchName) {

@sondermanish sondermanish Jan 1, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where are you putting all these methods?

@nidhi-nair nidhi-nair Jan 1, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They're not used, deleting what need not be modified

// TODO sanitise response for default IDs
return this.findByBranchNameAndBaseCollectionId(branchName, id, actionPermission.getReadPermission());
}

@Override
public Flux<ActionCollectionDTO> getPopulatedActionCollectionsByViewMode(
MultiValueMap<String, String> params, Boolean viewMode) {
return this.getNonComposedActionCollectionsByViewMode(params, viewMode)
.flatMap(actionCollectionDTO -> this.populateActionCollectionByViewMode(actionCollectionDTO, viewMode));
}

@Override
public Flux<ActionCollectionDTO> getPopulatedActionCollectionsByViewMode(
MultiValueMap<String, String> params, Boolean viewMode, String branchName) {
MultiValueMap<String, String> updatedMap = new LinkedMultiValueMap<>(params);
if (StringUtils.hasLength(branchName)) {
updatedMap.add(FieldName.BRANCH_NAME, branchName);
}
return this.getPopulatedActionCollectionsByViewMode(updatedMap, viewMode);
}

@Override
public Mono<ActionCollectionDTO> populateActionCollectionByViewMode(
ActionCollectionDTO actionCollectionDTO1, Boolean viewMode) {
Expand Down Expand Up @@ -459,23 +441,6 @@ protected Mono<ActionCollection> archiveGivenActionCollection(ActionCollection a
deletedActionCollection, getAnalyticsProperties(deletedActionCollection)));
}

@Override
public Mono<ActionCollection> findByBranchNameAndBaseCollectionId(
String branchName, String baseCollectionId, AclPermission permission) {

if (StringUtils.isEmpty(baseCollectionId)) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.COLLECTION_ID));
} else if (StringUtils.isEmpty(branchName)) {
return this.findById(baseCollectionId, permission)
.switchIfEmpty(Mono.error(new AppsmithException(
AppsmithError.NO_RESOURCE_FOUND, FieldName.ACTION_COLLECTION, baseCollectionId)));
}
return repository
.findByBranchNameAndBaseCollectionId(branchName, baseCollectionId, permission)
.switchIfEmpty(Mono.error(new AppsmithException(
AppsmithError.ACL_NO_RESOURCE_FOUND, FieldName.ACTION_COLLECTION, baseCollectionId)));
}

@Override
public Map<String, Object> getAnalyticsProperties(ActionCollection savedActionCollection) {
final ActionCollectionDTO unpublishedCollection = savedActionCollection.getUnpublishedCollection();
Expand All @@ -494,17 +459,6 @@ public Mono<ActionCollection> create(ActionCollection collection) {
return super.create(collection);
}

@Override
public Flux<ActionCollection> findAllActionCollectionsByContextIdAndContextTypeAndViewMode(
String contextId, CreatorContextType contextType, AclPermission permission, boolean viewMode) {
if (viewMode) {
return repository.findAllPublishedActionCollectionsByContextIdAndContextType(
contextId, contextType, permission);
}
return repository.findAllUnpublishedActionCollectionsByContextIdAndContextType(
contextId, contextType, permission);
}

protected Mono<ActionDTO> createJsAction(ActionCollection actionCollection, ActionDTO action) {
ActionCollectionDTO collectionDTO = actionCollection.getUnpublishedCollection();

Expand Down Expand Up @@ -546,7 +500,8 @@ protected Mono<ActionDTO> createJsAction(ActionCollection actionCollection, Acti

newAction.setPolicies(actionCollectionPolicies);
newActionService.setCommonFieldsFromActionDTOIntoNewAction(action, newAction);
newAction.setBranchName(actionCollection.getBranchName());
newAction.setRefType(actionCollection.getRefType());
newAction.setRefName(actionCollection.getRefName());

Mono<NewAction> sendAnalyticsMono =
analyticsService.sendCreateEvent(newAction, newActionService.getAnalyticsProperties(newAction));
Expand Down
3 changes: 2 additions & 1 deletion ...appsmith/server/actioncollections/importable/ActionCollectionImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,8 @@ private void updateExistingCollection(
populateDomainMappedReferences(mappedImportableResourcesDTO, existingActionCollection);

// Update branchName
existingActionCollection.setBranchName(importingMetaDTO.getBranchName());
existingActionCollection.setRefType(importingMetaDTO.getRefType());
existingActionCollection.setRefName(importingMetaDTO.getRefName());
// Recover the deleted state present in DB from imported actionCollection
existingActionCollection
.getUnpublishedCollection()
Expand Down
3 changes: 2 additions & 1 deletion ...llections/importable/applications/ActionCollectionApplicationImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ public void createNewResource(
// create or update base id for the action
// values already set to base id are kept unchanged
actionCollection.setBaseId(actionCollection.getBaseIdOrFallback());
actionCollection.setBranchName(importingMetaDTO.getBranchName());
actionCollection.setRefType(importingMetaDTO.getRefType());
actionCollection.setRefName(importingMetaDTO.getRefName());

// generate gitSyncId if it's not present
if (actionCollection.getGitSyncId() == null) {
Expand Down
11 changes: 7 additions & 4 deletions ...server/src/main/java/com/appsmith/server/applications/git/GitApplicationHelperCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ public Mono<Application> disconnectEntitiesOfBaseArtifact(Artifact baseArtifact)
.flatMap(page -> newPageService.findById(page.getId(), null))
.map(newPage -> {
newPage.setBaseId(newPage.getId());
newPage.setBranchName(null);
newPage.setRefType(null);
newPage.setRefName(null);
return newPage;
})
.collectList()
Expand All @@ -272,7 +273,8 @@ public Mono<Application> disconnectEntitiesOfBaseArtifact(Artifact baseArtifact)
.findByPageId(newPage.getId(), Optional.empty())
.map(newAction -> {
newAction.setBaseId(newAction.getId());
newAction.setBranchName(null);
newAction.setRefType(null);
newAction.setRefName(null);
return newAction;
})
.collectList()
Expand All @@ -284,7 +286,8 @@ public Mono<Application> disconnectEntitiesOfBaseArtifact(Artifact baseArtifact)
.findByPageId(newPage.getId())
.map(actionCollection -> {
actionCollection.setBaseId(actionCollection.getId());
actionCollection.setBranchName(null);
actionCollection.setRefType(null);
actionCollection.setRefName(null);
Comment on lines +289 to +290

@coderabbitai coderabbitai Bot Jan 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consolidate repetitive blocks.

You're setting refType and refName to null multiple times (pages, actions, and action collections). Extract these steps into a helper function (e.g. clearRefProperties(object)) to reduce code duplication.

- newPage.setRefType(null);
- newPage.setRefName(null);
- newAction.setRefType(null);
- newAction.setRefName(null);
- actionCollection.setRefType(null);
- actionCollection.setRefName(null);
+ clearRefProperties(newPage);
+ clearRefProperties(newAction);
+ clearRefProperties(actionCollection);

Committable suggestion skipped: line range outside the PR's diff.

return actionCollection;
})
.collectList()
Expand Down Expand Up @@ -334,7 +337,7 @@ public Mono<Application> publishArtifactPostRefCreation(
Artifact artifact, RefType refType, Boolean isPublishedManually) {
// TODO: create publish for ref type creation.
Application application = (Application) artifact;
if (RefType.TAG.equals(refType)) {
if (RefType.tag.equals(refType)) {
return Mono.just(application);
}

Expand Down
6 changes: 0 additions & 6 deletions ...rc/main/java/com/appsmith/server/applications/imports/ApplicationImportServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,12 +460,6 @@ public Mono<Application> updateAndSaveArtifactInContext(
}
}
return importApplicationMono
.doOnNext(application -> {
if (application.getGitArtifactMetadata() != null) {
Comment thread
nidhi-nair marked this conversation as resolved.
importingMetaDTO.setBranchName(
application.getGitArtifactMetadata().getBranchName());
}
})
.elapsed()
.map(tuples -> {
log.debug("time to create or update application object: {}", tuples.getT1());
Expand Down
37 changes: 28 additions & 9 deletions ...smith-server/src/main/java/com/appsmith/server/controllers/ConsolidatedAPIController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.server.controllers;

import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.views.Views;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.constants.Url;
Expand All @@ -11,6 +12,7 @@
import io.micrometer.observation.ObservationRegistry;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestMapping;
Expand Down Expand Up @@ -47,22 +49,30 @@ public ConsolidatedAPIController(
public Mono<ResponseDTO<ConsolidatedAPIResponseDTO>> getAllDataForFirstPageLoadForEditMode(
@RequestParam(name = FieldName.APPLICATION_ID, required = false) String baseApplicationId,
@RequestParam(name = "defaultPageId", required = false) String basePageId,
@RequestHeader(required = false, defaultValue = "branch") RefType refType,
@RequestHeader(required = false) String refName,
@RequestHeader(required = false) String branchName) {

if (!StringUtils.hasLength(refName)) {
refName = branchName;
}
log.debug(
"Going to fetch consolidatedAPI response for baseApplicationId: {}, basePageId: {}, branchName: {}, "
"Going to fetch consolidatedAPI response for baseApplicationId: {}, basePageId: {}, {}: {}, "
+ "mode: {}",
baseApplicationId,
basePageId,
branchName,
refType,
refName,
ApplicationMode.EDIT);

return consolidatedAPIService
.getConsolidatedInfoForPageLoad(basePageId, baseApplicationId, branchName, ApplicationMode.EDIT)
.getConsolidatedInfoForPageLoad(basePageId, baseApplicationId, refType, refName, ApplicationMode.EDIT)
.map(consolidatedAPIResponseDTO ->
new ResponseDTO<>(HttpStatus.OK.value(), consolidatedAPIResponseDTO, null))
.tag("pageId", Objects.toString(basePageId))
.tag("applicationId", Objects.toString(baseApplicationId))
.tag("branchName", Objects.toString(branchName))
.tag("refType", Objects.toString(refType))
.tag("refName", Objects.toString(refName))
.name(CONSOLIDATED_API_ROOT_EDIT)
.tap(Micrometer.observation(observationRegistry));
}
Expand All @@ -72,22 +82,31 @@ public Mono<ResponseDTO<ConsolidatedAPIResponseDTO>> getAllDataForFirstPageLoadF
public Mono<ResponseDTO<ConsolidatedAPIResponseDTO>> getAllDataForFirstPageLoadForViewMode(
@RequestParam(required = false) String applicationId,
@RequestParam(required = false) String defaultPageId,
@RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) {
@RequestHeader(required = false, defaultValue = "branch") RefType refType,
@RequestHeader(required = false) String refName,
@RequestHeader(required = false) String branchName) {

if (!StringUtils.hasLength(refName)) {
refName = branchName;
}
log.debug(
"Going to fetch consolidatedAPI response for applicationId: {}, defaultPageId: {}, branchName: {}, "
"Going to fetch consolidatedAPI response for applicationId: {}, defaultPageId: {}, {}: {}, "
+ "mode: {}",
applicationId,
defaultPageId,
branchName,
refType,
refName,
ApplicationMode.PUBLISHED);

return consolidatedAPIService
.getConsolidatedInfoForPageLoad(defaultPageId, applicationId, branchName, ApplicationMode.PUBLISHED)
.getConsolidatedInfoForPageLoad(
defaultPageId, applicationId, refType, refName, ApplicationMode.PUBLISHED)
.map(consolidatedAPIResponseDTO ->
new ResponseDTO<>(HttpStatus.OK.value(), consolidatedAPIResponseDTO, null))
.tag("pageId", Objects.toString(defaultPageId))
.tag("applicationId", Objects.toString(applicationId))
.tag("branchName", Objects.toString(branchName))
.tag("refType", Objects.toString(refType))
.tag("refName", Objects.toString(refName))
.name(CONSOLIDATED_API_ROOT_VIEW)
.tap(Micrometer.observation(observationRegistry));
}
Expand Down
Loading