fix: Take previously connected git applications accessibility into consideration to accurately calculate private repo count (#11151)

* Take already connected apps repo accessibility into account during connect and import flows

* Testcases for private repo limit check
This commit is contained in:
Abhijeet 2022-02-16 13:36:39 +05:30 committed by GitHub
parent 81e79af04f
commit 325f27982a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 228 additions and 64 deletions

View File

@ -42,5 +42,7 @@ public interface CustomApplicationRepositoryCE extends AppsmithRepository<Applic
Mono<Long> countByOrganizationId(String organizationId);
Mono<Long> getGitConnectedApplicationCount(String organizationId);
Mono<Long> getGitConnectedApplicationWithPrivateRepoCount(String organizationId);
Flux<Application> getGitConnectedApplicationByOrganizationId(String organizationId);
}

View File

@ -170,15 +170,26 @@ public class CustomApplicationRepositoryCEImpl extends BaseAppsmithRepositoryImp
}
@Override
public Mono<Long> getGitConnectedApplicationCount(String organizationId) {
public Mono<Long> getGitConnectedApplicationWithPrivateRepoCount(String organizationId) {
String gitApplicationMetadata = fieldName(QApplication.application.gitApplicationMetadata);
Query query = new Query();
query.addCriteria(where(fieldName(QApplication.application.organizationId)).is(organizationId));
query.addCriteria(where(gitApplicationMetadata + "." + fieldName(QApplication.application.gitApplicationMetadata.isRepoPrivate)).is(Boolean.TRUE));
query.addCriteria(where(fieldName(QApplication.application.deleted)).is(Boolean.FALSE));
query.addCriteria(notDeleted());
return mongoOperations.count(query, Application.class);
}
@Override
public Flux<Application> getGitConnectedApplicationByOrganizationId(String organizationId) {
String gitApplicationMetadata = fieldName(QApplication.application.gitApplicationMetadata);
// isRepoPrivate and gitAuth will be stored only with default application which ensures we will have only single
// application per repo
Criteria repoCriteria = where(gitApplicationMetadata + "." + fieldName(QApplication.application.gitApplicationMetadata.isRepoPrivate)).exists(Boolean.TRUE);
Criteria gitAuthCriteria = where(gitApplicationMetadata + "." + fieldName(QApplication.application.gitApplicationMetadata.gitAuth)).exists(Boolean.TRUE);
Criteria organizationIdCriteria = where(fieldName(QApplication.application.organizationId)).is(organizationId);
return queryAll(List.of(organizationIdCriteria, repoCriteria, gitAuthCriteria), AclPermission.MANAGE_APPLICATIONS);
}
@Override
public Mono<UpdateResult> setAppTheme(String applicationId, String editModeThemeId, String publishedModeThemeId, AclPermission aclPermission) {
Update updateObj = new Update();

View File

@ -56,7 +56,9 @@ public interface ApplicationServiceCE extends CrudService<Application, String> {
Flux<Application> findAllApplicationsByDefaultApplicationId(String defaultApplicationId);
Mono<Long> getGitConnectedApplicationCount(String organizationId);
Mono<Long> getGitConnectedApplicationsCountWithPrivateRepoByOrgId(String organizationId);
Flux<Application> getGitConnectedApplicationsByOrganizationId(String organizationId);
String getRandomAppCardColor();

View File

@ -533,8 +533,13 @@ public class ApplicationServiceCEImpl extends BaseService<ApplicationRepository,
}
@Override
public Mono<Long> getGitConnectedApplicationCount(String organizationId) {
return repository.getGitConnectedApplicationCount(organizationId);
public Mono<Long> getGitConnectedApplicationsCountWithPrivateRepoByOrgId(String organizationId) {
return repository.getGitConnectedApplicationWithPrivateRepoCount(organizationId);
}
@Override
public Flux<Application> getGitConnectedApplicationsByOrganizationId(String organizationId) {
return repository.getGitConnectedApplicationByOrganizationId(organizationId);
}
public String getRandomAppCardColor() {

View File

@ -389,39 +389,41 @@ public class GitServiceCEImpl implements GitServiceCE {
if (Optional.ofNullable(defaultGitMetadata).isEmpty()) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_CONFIG_ERROR));
}
Boolean dbIsRepoPrivate = defaultGitMetadata.getIsRepoPrivate();
if (!Boolean.TRUE.equals(dbIsRepoPrivate)) {
if (StringUtils.isEmptyOrNull(defaultGitMetadata.getBrowserSupportedRemoteUrl())) {
defaultGitMetadata.setBrowserSupportedRemoteUrl(
GitUtils.convertSshUrlToBrowserSupportedUrl(defaultGitMetadata.getRemoteUrl())
);
}
Boolean isRepoPrivateCurrentStatus = true;
// Check if the repo is public for current application and if the user have changed the access after
// the connection
final Boolean isRepoPrivate = defaultGitMetadata.getIsRepoPrivate();
Mono<Application> applicationMono = Mono.just(defaultApplication);
if (Boolean.FALSE.equals(isRepoPrivate)) {
try {
isRepoPrivateCurrentStatus = GitUtils.isRepoPrivate(defaultGitMetadata.getBrowserSupportedRemoteUrl());
}
catch (IOException e) {
defaultGitMetadata.setIsRepoPrivate(
GitUtils.isRepoPrivate(defaultGitMetadata.getBrowserSupportedRemoteUrl())
);
if (!isRepoPrivate.equals(defaultGitMetadata.getIsRepoPrivate())) {
applicationMono = applicationService.save(defaultApplication);
} else {
return applicationMono;
}
} catch (IOException e) {
log.debug("Error while checking if the repo is private: ", e);
}
if (!isRepoPrivateCurrentStatus.equals(dbIsRepoPrivate)) {
defaultGitMetadata.setIsRepoPrivate(isRepoPrivateCurrentStatus);
// check if the commit application will be allowed if the repo is made private
return applicationService.save(defaultApplication)
//Check the limit for number of private repo
.flatMap(application -> gitCloudServicesUtils.getPrivateRepoLimitForOrg(application.getOrganizationId(), false)
.flatMap(limitCount -> {
//get git connected apps count from db
return applicationService.getGitConnectedApplicationCount(application.getOrganizationId())
.flatMap(count -> {
if (limitCount <= count) {
return Mono.error(new AppsmithException(AppsmithError.GIT_APPLICATION_LIMIT_ERROR));
}
return Mono.just(application);
});
}));
}
}
return Mono.just(defaultApplication);
// Check if the private repo count is less than the allowed repo count
final String orgId = defaultApplication.getOrganizationId();
return applicationMono
.then(gitCloudServicesUtils.getPrivateRepoLimitForOrg(orgId, false))
.flatMap(limit -> {
if (limit == -1) {
return Mono.just(defaultApplication);
}
return this.getApplicationCountWithPrivateRepo(orgId)
.map(privateRepoCount -> {
if (limit >= privateRepoCount) {
return defaultApplication;
}
throw new AppsmithException(AppsmithError.GIT_APPLICATION_LIMIT_ERROR);
});
});
})
.then(applicationService.findByBranchNameAndDefaultApplicationId(branchName, defaultApplicationId, MANAGE_APPLICATIONS))
.flatMap(branchedApplication -> publishAndOrGetApplication(branchedApplication.getId(), commitDTO.getDoPush()))
@ -630,24 +632,33 @@ public class GitServiceCEImpl implements GitServiceCE {
Mono<Map<String, GitProfile>> profileMono = updateOrCreateGitProfileForCurrentUser(gitConnectDTO.getGitProfile(), defaultApplicationId)
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_CONFIGURATION, GIT_PROFILE_ERROR)));
final String browserSupportedUrl = GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl());
boolean isRepoPrivateTemp = true;
try {
isRepoPrivateTemp = GitUtils.isRepoPrivate(browserSupportedUrl);
} catch (IOException e) {
log.debug("Error while checking if the repo is private: ", e);
}
final boolean isRepoPrivate = isRepoPrivateTemp;
Mono<Application> connectApplicationMono = profileMono
.then(getApplicationById(defaultApplicationId))
.flatMap(application -> {
// Check if the repo is public
try {
if(!GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl()))) {
return Mono.just(application);
}
} catch (IOException e) {
log.debug("Error while checking if the repo is private: ", e);
if(!isRepoPrivate) {
return Mono.just(application);
}
//Check the limit for number of private repo
return gitCloudServicesUtils.getPrivateRepoLimitForOrg(application.getOrganizationId(), true)
.flatMap(limitCount -> {
// CS will respond with count -1 for unlimited git repos
if (limitCount == -1) {
return Mono.just(application);
}
// get git connected apps count from db
return applicationService.getGitConnectedApplicationCount(application.getOrganizationId())
return this.getApplicationCountWithPrivateRepo(application.getOrganizationId())
.flatMap(count -> {
if (limitCount < count) {
if (limitCount <= count) {
return addAnalyticsForGitOperation(
AnalyticsEvents.GIT_PRIVATE_REPO_LIMIT_EXCEEDED.getEventName(),
application,
@ -655,7 +666,7 @@ public class GitServiceCEImpl implements GitServiceCE {
AppsmithError.GIT_APPLICATION_LIMIT_ERROR.getMessage(),
application.getGitApplicationMetadata().getIsRepoPrivate()
)
.flatMap(user -> Mono.error(new AppsmithException(AppsmithError.GIT_APPLICATION_LIMIT_ERROR)));
.flatMap(ignore -> Mono.error(new AppsmithException(AppsmithError.GIT_APPLICATION_LIMIT_ERROR)));
}
return Mono.just(application);
});
@ -730,17 +741,9 @@ public class GitServiceCEImpl implements GitServiceCE {
gitApplicationMetadata.setDefaultBranchName(defaultBranch);
gitApplicationMetadata.setRemoteUrl(gitConnectDTO.getRemoteUrl());
gitApplicationMetadata.setRepoName(repoName);
gitApplicationMetadata.setBrowserSupportedRemoteUrl(
GitUtils.convertSshUrlToBrowserSupportedUrl(gitConnectDTO.getRemoteUrl())
);
try {
gitApplicationMetadata.setIsRepoPrivate(
GitUtils.isRepoPrivate(gitApplicationMetadata.getBrowserSupportedRemoteUrl())
);
} catch (IOException e) {
gitApplicationMetadata.setIsRepoPrivate(true);
log.debug("Error while checking if the repo is private: ", e);
}
gitApplicationMetadata.setBrowserSupportedRemoteUrl(browserSupportedUrl);
gitApplicationMetadata.setIsRepoPrivate(isRepoPrivate);
// Set branchName for each application resource
return importExportApplicationService.exportApplicationById(applicationId, SerialiseApplicationObjective.VERSION_CONTROL)
@ -1925,10 +1928,14 @@ public class GitServiceCEImpl implements GitServiceCE {
}
return gitCloudServicesUtils.getPrivateRepoLimitForOrg(organizationId, true)
.flatMap(limitCount -> {
// CS will respond with count -1 for unlimited git repos
if (limitCount == -1) {
return Mono.just(gitAuth).zipWith(applicationMono);
}
// get git connected apps count from db
return applicationService.getGitConnectedApplicationCount(organizationId)
return this.getApplicationCountWithPrivateRepo(organizationId)
.flatMap(count -> {
if (limitCount < count) {
if (limitCount <= count) {
return addAnalyticsForGitOperation(
AnalyticsEvents.GIT_IMPORT.getEventName(),
newApplication,
@ -2224,6 +2231,28 @@ public class GitServiceCEImpl implements GitServiceCE {
});
}
private Mono<Long> getApplicationCountWithPrivateRepo(String organizationId) {
return applicationService.getGitConnectedApplicationsByOrganizationId(organizationId)
.flatMap(application -> {
GitApplicationMetadata gitData = application.getGitApplicationMetadata();
final Boolean isRepoPrivate = gitData.getIsRepoPrivate();
try {
// Check if user have altered the repo accessibility
gitData.setIsRepoPrivate(
GitUtils.isRepoPrivate(application.getGitApplicationMetadata().getBrowserSupportedRemoteUrl())
);
if (!isRepoPrivate.equals(gitData.getIsRepoPrivate())) {
// Repo accessibility is changed
return applicationService.save(application);
}
} catch (IOException e) {
log.debug("Error while checking if the repo is private: ", e);
}
return Mono.just(application);
})
.then(applicationService.getGitConnectedApplicationsCountWithPrivateRepoByOrgId(organizationId));
}
private Mono<Application> addAnalyticsForGitOperation(String eventName, Application application, Boolean isRepoPrivate) {
return addAnalyticsForGitOperation(eventName, application, "", "", isRepoPrivate);
}

View File

@ -161,9 +161,15 @@ public class GitServiceTest {
@Before
public void setup() throws IOException {
if (StringUtils.isEmpty(orgId)) {
orgId = organizationRepository
.findByName("Another Test Organization", AclPermission.READ_ORGANIZATIONS)
.block()
.getId();
}
Mockito
.when(gitCloudServicesUtils.getPrivateRepoLimitForOrg(Mockito.any(), Mockito.anyBoolean()))
.thenReturn(Mono.just(100));
.when(gitCloudServicesUtils.getPrivateRepoLimitForOrg(eq(orgId), Mockito.anyBoolean()))
.thenReturn(Mono.just(-1));
Mockito
.when(pluginExecutorHelper.getPluginExecutor(Mockito.any()))
@ -172,8 +178,6 @@ public class GitServiceTest {
if (Boolean.TRUE.equals(isSetupDone)) {
return;
}
Organization testOrg = organizationRepository.findByName("Another Test Organization", AclPermission.READ_ORGANIZATIONS).block();
orgId = testOrg.getId();
gitConnectedApplication = createApplicationConnectedToGit("gitConnectedApplication", DEFAULT_BRANCH);
@ -222,6 +226,10 @@ public class GitServiceTest {
}
private Application createApplicationConnectedToGit(String name, String branchName) throws IOException {
return createApplicationConnectedToGit(name, branchName, orgId);
}
private Application createApplicationConnectedToGit(String name, String branchName, String organizationId) throws IOException {
if (StringUtils.isEmpty(branchName)) {
branchName = DEFAULT_BRANCH;
@ -245,7 +253,7 @@ public class GitServiceTest {
Application testApplication = new Application();
testApplication.setName(name);
testApplication.setOrganizationId(orgId);
testApplication.setOrganizationId(organizationId);
Application application1 = applicationPageService.createApplication(testApplication).block();
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
@ -263,7 +271,8 @@ public class GitServiceTest {
page.setApplicationId(application1.getId());
applicationPageService.createPage(page).block();
GitConnectDTO gitConnectDTO = getConnectRequest("git@github.com:test/testRepo.git", testUserProfile);
String repoUrl = String.format("git@github.com:test/%s.git", name);
GitConnectDTO gitConnectDTO = getConnectRequest(repoUrl, testUserProfile);
return gitService.connectApplicationToGit(application1.getId(), gitConnectDTO, "baseUrl").block();
}
@ -564,7 +573,7 @@ public class GitServiceTest {
@Test
@WithUserDetails(value = "api_user")
public void connectApplicationToGit_WithoutGitProfileUsingLocalProfile_ThrowAuthorNameUnavailableError() throws IOException {
public void connectApplicationToGit_WithoutGitProfileUsingLocalProfile_ThrowAuthorNameUnavailableError() {
GitProfile gitProfile = new GitProfile();
gitProfile.setAuthorName(null);
@ -648,6 +657,109 @@ public class GitServiceTest {
.verifyComplete();
}
@Test
@WithUserDetails(value = "api_user")
public void connectApplicationToGit_moreThanThreePrivateRepos_throwException() throws IOException {
Organization organization = new Organization();
organization.setName("Limit Private Repo Test Organization");
String limitPrivateRepoTestOrgId = organizationService.create(organization).map(Organization::getId).block();
Mockito
.when(gitCloudServicesUtils.getPrivateRepoLimitForOrg(eq(limitPrivateRepoTestOrgId), Mockito.anyBoolean()))
.thenReturn(Mono.just(3));
Mockito.when(gitExecutor.cloneApplication(Mockito.any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just("defaultBranchName"));
Mockito.when(gitExecutor.commitApplication(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()))
.thenReturn(Mono.just("commit"));
Mockito.when(gitExecutor.checkoutToBranch(Mockito.any(Path.class), Mockito.anyString())).thenReturn(Mono.just(true));
Mockito.when(gitExecutor.pushApplication(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(),
Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just("success"));
Mockito.when(gitFileUtils.checkIfDirectoryIsEmpty(Mockito.any(Path.class))).thenReturn(Mono.just(true));
Mockito.when(gitFileUtils.initializeGitRepo(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just(Paths.get("textPath")));
this.createApplicationConnectedToGit("private_repo_1", "master", limitPrivateRepoTestOrgId);
this.createApplicationConnectedToGit("private_repo_2", "master", limitPrivateRepoTestOrgId);
this.createApplicationConnectedToGit("private_repo_3", "master", limitPrivateRepoTestOrgId);
Application testApplication = new Application();
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
GitAuth gitAuth = new GitAuth();
gitAuth.setPublicKey("testkey");
gitAuth.setPrivateKey("privatekey");
gitAuth.setGeneratedAt(Instant.now());
gitAuth.setDocUrl("docUrl");
gitApplicationMetadata.setGitAuth(gitAuth);
testApplication.setGitApplicationMetadata(gitApplicationMetadata);
testApplication.setName("connectApplicationToGit_WithNonEmptyPublishedPages");
testApplication.setOrganizationId(limitPrivateRepoTestOrgId);
Application application = applicationPageService.createApplication(testApplication).block();
GitConnectDTO gitConnectDTO = getConnectRequest("git@github.com:test/testRepo.git", testUserProfile);
Mono<Application> applicationMono = gitService.connectApplicationToGit(application.getId(), gitConnectDTO, "baseUrl");
StepVerifier
.create(applicationMono)
.expectErrorMatches(error -> error instanceof AppsmithException
&& error.getMessage().equals(AppsmithError.GIT_APPLICATION_LIMIT_ERROR.getMessage()))
.verify();
}
@Test
@WithUserDetails(value = "api_user")
public void connectApplicationToGit_toggleAccessibilityToPublicForConnectedApp_connectSuccessful() throws IOException {
Organization organization = new Organization();
organization.setName("Toggle Accessibility To Public From Private Repo Test Organization");
String limitPrivateRepoTestOrgId = organizationService.create(organization).map(Organization::getId).block();
Mockito
.when(gitCloudServicesUtils.getPrivateRepoLimitForOrg(eq(limitPrivateRepoTestOrgId), Mockito.anyBoolean()))
.thenReturn(Mono.just(3));
Mockito.when(gitExecutor.cloneApplication(Mockito.any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just("defaultBranchName"));
Mockito.when(gitExecutor.commitApplication(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()))
.thenReturn(Mono.just("commit"));
Mockito.when(gitExecutor.checkoutToBranch(Mockito.any(Path.class), Mockito.anyString())).thenReturn(Mono.just(true));
Mockito.when(gitExecutor.pushApplication(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(),
Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just("success"));
Mockito.when(gitFileUtils.checkIfDirectoryIsEmpty(Mockito.any(Path.class))).thenReturn(Mono.just(true));
Mockito.when(gitFileUtils.initializeGitRepo(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just(Paths.get("textPath")));
Application application1 = this.createApplicationConnectedToGit("private_repo_1", "master", limitPrivateRepoTestOrgId);
this.createApplicationConnectedToGit("private_repo_2", "master", limitPrivateRepoTestOrgId);
this.createApplicationConnectedToGit("private_repo_3", "master", limitPrivateRepoTestOrgId);
Application testApplication = new Application();
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
GitAuth gitAuth = new GitAuth();
gitAuth.setPublicKey("testkey");
gitAuth.setPrivateKey("privatekey");
gitAuth.setGeneratedAt(Instant.now());
gitAuth.setDocUrl("docUrl");
gitApplicationMetadata.setGitAuth(gitAuth);
testApplication.setGitApplicationMetadata(gitApplicationMetadata);
testApplication.setName("connectApplicationToGit_WithNonEmptyPublishedPages");
testApplication.setOrganizationId(limitPrivateRepoTestOrgId);
Application application = applicationPageService.createApplication(testApplication).block();
GitConnectDTO gitConnectDTO = getConnectRequest("git@github.com:test/testRepo.git", testUserProfile);
Mono<Application> applicationMono = gitService.connectApplicationToGit(application.getId(), gitConnectDTO, "baseUrl");
// Use any dummy url so as to get 2xx response
application1.getGitApplicationMetadata().setBrowserSupportedRemoteUrl("https://www.google.com/");
applicationService.save(application1).block();
StepVerifier
.create(applicationMono)
.assertNext(connectedApp -> {
assertThat(connectedApp.getId()).isNotEmpty();
})
.verifyComplete();
}
@Test
@WithUserDetails(value = "api_user")
public void updateGitMetadata_EmptyData_Success() {
@ -2325,6 +2437,9 @@ public class GitServiceTest {
datasource.setOrganizationId(testOrgId);
datasourceService.create(datasource).block();
Mockito
.when(gitCloudServicesUtils.getPrivateRepoLimitForOrg(eq(testOrgId), Mockito.anyBoolean()))
.thenReturn(Mono.just(3));
Mockito.when(gitExecutor.cloneApplication(Mockito.any(Path.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()))
.thenReturn(Mono.just("defaultBranch"));
Mockito.when(gitFileUtils.reconstructApplicationJsonFromGitRepo(Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString()))