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.

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

chore: Add code-split for Git integration for packages #39166

Merged
merged 3 commits into from
Feb 25, 2025
+87 −66
Merged

chore: Add code-split for Git integration for packages #39166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e23ae6
Add code-split for Git integration for packages
subrata71 Feb 11, 2025
b2f06cc
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
subrata71 Feb 11, 2025
57eb731
Merge branch 'release' of github.com:appsmithorg/appsmith into chore/…
subrata71 Feb 25, 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 ...ppsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2259,7 +2259,7 @@ protected Mono<? extends Artifact> discardChanges(Artifact branchedArtifact, Git
.flatMap(artifactExchangeJson -> importService.importArtifactInWorkspaceFromGit(
workspaceId, branchedArtifact.getId(), artifactExchangeJson, branchName))
.flatMap(artifactFromLastCommit ->
gitArtifactHelper.publishArtifact(artifactFromLastCommit, true))
gitArtifactHelper.validateAndPublishArtifact(artifactFromLastCommit, true))
.flatMap(publishedArtifact -> gitAnalyticsUtils.addAnalyticsForGitOperation(
AnalyticsEvents.GIT_DISCARD_CHANGES, publishedArtifact, null))
.onErrorResume(exception -> {
Expand Down
53 changes: 53 additions & 0 deletions ...erver/appsmith-server/src/test/utils/com/appsmith/server/git/GitApplicationTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.appsmith.server.git;

import com.appsmith.external.constants.PluginConstants;
import com.appsmith.external.models.ActionConfiguration;
import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.PluginType;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.Artifact;
import com.appsmith.server.domains.Plugin;
import com.appsmith.server.plugins.base.PluginService;
import com.appsmith.server.services.LayoutActionService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpMethod;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import java.util.UUID;

@Component
public class GitApplicationTestUtils implements GitArtifactTestUtils<Application> {

@Autowired
LayoutActionService layoutActionService;
@Autowired
PluginService pluginService;
Comment on lines +23 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace field injection with constructor injection.

Field injection is not recommended. Use constructor injection instead for better testability and explicit dependency declaration.

-    @Autowired
-    LayoutActionService layoutActionService;
-    @Autowired
-    PluginService pluginService;
+    private final LayoutActionService layoutActionService;
+    private final PluginService pluginService;
+
+    public GitApplicationTestUtils(
+            LayoutActionService layoutActionService,
+            PluginService pluginService) {
+        this.layoutActionService = layoutActionService;
+        this.pluginService = pluginService;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Autowired
LayoutActionService layoutActionService;
@Autowired
PluginService pluginService;
private final LayoutActionService layoutActionService;
private final PluginService pluginService;
public GitApplicationTestUtils(
LayoutActionService layoutActionService,
PluginService pluginService) {
this.layoutActionService = layoutActionService;
this.pluginService = pluginService;
}



@Override
public Mono<Void> createADiff(Artifact artifact) {

Application application = (Application) artifact;
String pageId = application.getPages().get(0).getId();
Plugin plugin = pluginService.findByPackageName("restapi-plugin").block();
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid blocking calls in reactive code.

The .block() call breaks the reactive chain and could cause performance issues. Consider using reactive operators instead.

-        Plugin plugin = pluginService.findByPackageName("restapi-plugin").block();
+        return pluginService.findByPackageName("restapi-plugin")
+            .flatMap(plugin -> {
+                Datasource datasource = new Datasource();
+                // ... rest of the code
+                return layoutActionService.createSingleAction(action, Boolean.FALSE);
+            })
+            .then();

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


Datasource datasource = new Datasource();
datasource.setName(PluginConstants.DEFAULT_REST_DATASOURCE);
datasource.setWorkspaceId(application.getWorkspaceId());
datasource.setPluginId(plugin.getId());

ActionDTO action = new ActionDTO();
action.setPluginType(PluginType.API);
action.setName("aGetAction_" + UUID.randomUUID());
action.setDatasource(datasource);
action.setActionConfiguration(new ActionConfiguration());
action.getActionConfiguration().setHttpMethod(HttpMethod.GET);
action.setPageId(pageId);

return layoutActionService
.createSingleAction(action, Boolean.FALSE)
.then();
}
}
48 changes: 2 additions & 46 deletions app/server/appsmith-server/src/test/utils/com/appsmith/server/git/GitArtifactTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,52 +1,8 @@
package com.appsmith.server.git;

import com.appsmith.external.constants.PluginConstants;
import com.appsmith.external.models.ActionConfiguration;
import com.appsmith.external.models.ActionDTO;
import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.PluginType;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.Artifact;
import com.appsmith.server.domains.Plugin;
import com.appsmith.server.plugins.base.PluginService;
import com.appsmith.server.services.LayoutActionService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpMethod;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import java.util.UUID;

@Component
public class GitArtifactTestUtils<T extends Artifact> {

@Autowired
LayoutActionService layoutActionService;
@Autowired
PluginService pluginService;

Mono<Void> createADiff(Artifact artifact) {

Application application = (Application) artifact;

String pageId = application.getPages().get(0).getId();
Plugin plugin = pluginService.findByPackageName("restapi-plugin").block();

Datasource datasource = new Datasource();
datasource.setName(PluginConstants.DEFAULT_REST_DATASOURCE);
datasource.setWorkspaceId(application.getWorkspaceId());
datasource.setPluginId(plugin.getId());

ActionDTO action = new ActionDTO();
action.setPluginType(PluginType.API);
action.setName("aGetAction_" + UUID.randomUUID());
action.setDatasource(datasource);
action.setActionConfiguration(new ActionConfiguration());
action.getActionConfiguration().setHttpMethod(HttpMethod.GET);
action.setPageId(pageId);

return layoutActionService
.createSingleAction(action, Boolean.FALSE)
.then();
}
public interface GitArtifactTestUtils<T extends Artifact> {
Mono<Void> createADiff(Artifact artifact);
}
24 changes: 5 additions & 19 deletions app/server/appsmith-server/src/test/utils/com/appsmith/server/git/GitTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,13 @@
package com.appsmith.server.git;

import com.appsmith.server.constants.ArtifactType;

import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.Artifact;
import lombok.RequiredArgsConstructor;
import com.appsmith.server.git.ce.GitTestUtilsCE;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

@RequiredArgsConstructor
@Component
public class GitTestUtils {

private final GitArtifactTestUtils<Application> gitApplicationTestUtils;

private GitArtifactTestUtils<?> getArtifactSpecificUtils(ArtifactType artifactType) {
// TODO For now just work with apps
return gitApplicationTestUtils;
}


public Mono<Void> createADiffInArtifact(Artifact artifact) {
GitArtifactTestUtils<?> artifactSpecificUtils = getArtifactSpecificUtils(artifact.getArtifactType());

return artifactSpecificUtils.createADiff(artifact);
public class GitTestUtils extends GitTestUtilsCE {
public GitTestUtils(GitArtifactTestUtils<Application> gitApplicationTestUtils) {
super(gitApplicationTestUtils);
}
}
26 changes: 26 additions & 0 deletions app/server/appsmith-server/src/test/utils/com/appsmith/server/git/ce/GitTestUtilsCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.appsmith.server.git.ce;

import com.appsmith.server.constants.ArtifactType;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.Artifact;
import com.appsmith.server.git.GitArtifactTestUtils;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

@RequiredArgsConstructor
@Component
public class GitTestUtilsCE {

private final GitArtifactTestUtils<Application> gitApplicationTestUtils;
protected GitArtifactTestUtils<?> getArtifactSpecificUtils(ArtifactType artifactType) {
return gitApplicationTestUtils;
}


public Mono<Void> createADiffInArtifact(Artifact artifact) {
GitArtifactTestUtils<?> artifactSpecificUtils = getArtifactSpecificUtils(artifact.getArtifactType());

return artifactSpecificUtils.createADiff(artifact);
}
}