Skip to content

chore: Deprecate .findById signature with Optional parameter #34281

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
sharat87 merged 3 commits into from
Jun 18, 2024
Merged

chore: Deprecate .findById signature with Optional parameter #34281

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
d2edf14
Deprecate `.findById` signature with Optional parameter
sharat87 Jun 17, 2024
be4c02e
fix tests
sharat87 Jun 17, 2024
c1e45f2
Merge branch 'release' into chore/no-optional
sharat87 Jun 17, 2024
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
7 changes: 2 additions & 5 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
Expand Up @@ -14,7 +14,6 @@
import reactor.core.publisher.Mono;

import java.util.List;
import java.util.Optional;

public interface ActionCollectionServiceCE extends CrudService<ActionCollection, String> {

Expand Down Expand Up @@ -47,10 +46,8 @@ Mono<ActionCollectionDTO> splitValidActionsByViewMode(

Mono<ActionCollectionDTO> deleteUnpublishedActionCollection(String id);

Mono<ActionCollectionDTO> deleteUnpublishedActionCollectionWithOptionalPermission(
String id,
Optional<AclPermission> deleteCollectionPermission,
Optional<AclPermission> deleteActionPermission);
Mono<ActionCollectionDTO> deleteUnpublishedActionCollection(
String id, AclPermission deleteCollectionPermission, AclPermission deleteActionPermission);

Mono<ActionCollectionDTO> deleteWithoutPermissionUnpublishedActionCollection(String id);

Expand Down
23 changes: 6 additions & 17 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
Expand Up @@ -42,7 +42,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;

Expand Down Expand Up @@ -343,28 +342,18 @@ public Mono<ActionCollectionDTO> update(String id, ActionCollectionDTO actionCol

@Override
public Mono<ActionCollectionDTO> deleteWithoutPermissionUnpublishedActionCollection(String id) {
return deleteUnpublishedActionCollectionEx(
id, Optional.empty(), Optional.of(actionPermission.getDeletePermission()));
return deleteUnpublishedActionCollection(id, null, actionPermission.getDeletePermission());
}

@Override
public Mono<ActionCollectionDTO> deleteUnpublishedActionCollection(String id) {
return deleteUnpublishedActionCollectionEx(
id,
Optional.of(actionPermission.getDeletePermission()),
Optional.of(actionPermission.getDeletePermission()));
return deleteUnpublishedActionCollection(
id, actionPermission.getDeletePermission(), actionPermission.getDeletePermission());
}

@Override
public Mono<ActionCollectionDTO> deleteUnpublishedActionCollectionWithOptionalPermission(
String id,
Optional<AclPermission> deleteCollectionPermission,
Optional<AclPermission> deleteActionPermission) {
return deleteUnpublishedActionCollectionEx(id, deleteCollectionPermission, deleteActionPermission);
}

public Mono<ActionCollectionDTO> deleteUnpublishedActionCollectionEx(
String id, Optional<AclPermission> permission, Optional<AclPermission> deleteActionPermission) {
public Mono<ActionCollectionDTO> deleteUnpublishedActionCollection(
String id, AclPermission permission, AclPermission deleteActionPermission) {
Mono<ActionCollection> actionCollectionMono = repository
.findById(id, permission)
.switchIfEmpty(Mono.error(
Expand All @@ -377,7 +366,7 @@ public Mono<ActionCollectionDTO> deleteUnpublishedActionCollectionEx(
&& toDelete.getPublishedCollection().getName() != null) {
toDelete.getUnpublishedCollection().setDeletedAt(Instant.now());
modifiedActionCollectionMono = newActionService
.findByCollectionIdAndViewMode(id, false, deleteActionPermission.orElse(null))
.findByCollectionIdAndViewMode(id, false, deleteActionPermission)
.flatMap(newAction -> newActionService
.deleteGivenNewAction(newAction)
// return an empty action so that the filter can remove it from the list
Expand Down
2 changes: 1 addition & 1 deletion ...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 @@ -256,7 +256,7 @@ public Mono<Application> disconnectEntitiesOfDefaultArtifact(Artifact defaultArt
// Update all the resources to replace defaultResource Ids with the resource Ids as branchName
// will be deleted
Flux<NewPage> newPageFlux = Flux.fromIterable(defaultApplication.getPages())
.flatMap(page -> newPageService.findById(page.getId(), Optional.empty()))
.flatMap(page -> newPageService.findById(page.getId(), null))
.map(newPage -> {
newPage.setDefaultResources(null);
return createDefaultIdsOrUpdateWithGivenResourceIds(newPage, null);
Expand Down
3 changes: 1 addition & 2 deletions ...rc/main/java/com/appsmith/server/imports/internal/partial/PartialImportServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -375,7 +374,7 @@ private Mono<String> paneNameMapForActionAndActionCollectionInAppJson(
ApplicationJson applicationJson,
MappedImportableResourcesDTO mappedImportableResourcesDTO) {
return branchedPageIdMono.flatMap(
pageId -> newPageService.findById(pageId, Optional.empty()).flatMap(newPage -> {
pageId -> newPageService.findById(pageId, null).flatMap(newPage -> {
String pageName = newPage.getUnpublishedPage().getName();
// update page name reference with newPage
Map<String, NewPage> pageNameMap = new HashMap<>();
Expand Down
5 changes: 2 additions & 3 deletions ...appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public interface NewActionServiceCE extends CrudService<NewAction, String> {
Mono<ActionDTO> updateUnpublishedAction(String id, ActionDTO action);

Mono<Tuple2<ActionDTO, NewAction>> updateUnpublishedActionWithoutAnalytics(
String id, ActionDTO action, Optional<AclPermission> permission);
String id, ActionDTO action, AclPermission permission);

Mono<ActionDTO> findByUnpublishedNameAndPageId(String name, String pageId, AclPermission permission);

Expand Down Expand Up @@ -84,8 +84,7 @@ Flux<NewAction> findAllByApplicationIdAndViewMode(

Mono<ActionDTO> deleteUnpublishedAction(String id);

Mono<ActionDTO> deleteUnpublishedActionWithOptionalPermission(
String id, Optional<AclPermission> newActionDeletePermission);
Mono<ActionDTO> deleteUnpublishedAction(String id, AclPermission newActionDeletePermission);

Flux<ActionDTO> getUnpublishedActions(MultiValueMap<String, String> params, Boolean includeJsActions);

Expand Down
9 changes: 4 additions & 5 deletions ...mith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ public Mono<ActionDTO> updateUnpublishedAction(String id, ActionDTO action) {
action != null ? action.getId() : null,
id);

return updateUnpublishedActionWithoutAnalytics(id, action, Optional.of(actionPermission.getEditPermission()))
return updateUnpublishedActionWithoutAnalytics(id, action, actionPermission.getEditPermission())
.zipWhen(zippedActions -> {
ActionDTO updatedActionDTO = zippedActions.getT1();
if (updatedActionDTO.getDatasource() != null
Expand Down Expand Up @@ -626,7 +626,7 @@ public Mono<ActionDTO> updateUnpublishedAction(String id, ActionDTO action) {
*/
@Override
public Mono<Tuple2<ActionDTO, NewAction>> updateUnpublishedActionWithoutAnalytics(
String id, ActionDTO action, Optional<AclPermission> permission) {
String id, ActionDTO action, AclPermission permission) {
log.debug(
"Updating unpublished action without analytics with action id: {} ",
action != null ? action.getId() : null);
Expand Down Expand Up @@ -848,12 +848,11 @@ public ActionViewDTO generateActionViewDTO(NewAction action, ActionDTO actionDTO

@Override
public Mono<ActionDTO> deleteUnpublishedAction(String id) {
return deleteUnpublishedActionWithOptionalPermission(id, Optional.of(actionPermission.getDeletePermission()));
return deleteUnpublishedAction(id, actionPermission.getDeletePermission());
}

@Override
public Mono<ActionDTO> deleteUnpublishedActionWithOptionalPermission(
String id, Optional<AclPermission> newActionDeletePermission) {
public Mono<ActionDTO> deleteUnpublishedAction(String id, AclPermission newActionDeletePermission) {
Mono<NewAction> actionMono = repository
.findById(id, newActionDeletePermission)
.switchIfEmpty(
Expand Down
3 changes: 1 addition & 2 deletions ...main/java/com/appsmith/server/newactions/importable/NewActionImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties;
Expand Down Expand Up @@ -99,7 +98,7 @@ public Mono<Void> importEntities(
log.info("Deleting {} actions which are no more used", invalidActionIds.size());
return Flux.fromIterable(invalidActionIds)
.flatMap(actionId -> newActionService
.deleteUnpublishedActionWithOptionalPermission(actionId, Optional.empty())
.deleteUnpublishedAction(actionId, null)
// return an empty action so that the filter can remove it from the list
.onErrorResume(throwable -> {
log.debug("Failed to delete action with id {} during import", actionId);
Expand Down
4 changes: 1 addition & 3 deletions ...ver/appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ public interface NewPageServiceCE extends CrudService<NewPage, String> {

Mono<NewPage> findById(String pageId, AclPermission aclPermission);

Mono<NewPage> findById(String pageId, Optional<AclPermission> aclPermission);

Mono<PageDTO> findPageById(String pageId, AclPermission aclPermission, Boolean view);

Flux<PageDTO> findByApplicationId(String applicationId, AclPermission permission, Boolean view);
Expand Down Expand Up @@ -73,7 +71,7 @@ Mono<PageDTO> findByNameAndApplicationIdAndViewMode(

Mono<Boolean> archiveByIds(Collection<String> idList);

Mono<NewPage> archiveWithoutPermissionById(String id);
Mono<NewPage> archiveByIdWithoutPermission(String id);

Flux<NewPage> saveAll(List<NewPage> pages);

Expand Down
15 changes: 5 additions & 10 deletions ...appsmith-server/src/main/java/com/appsmith/server/newpages/base/NewPageServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,6 @@ public Mono<PageDTO> findPageById(String pageId, AclPermission aclPermission, Bo
return this.findById(pageId, aclPermission).flatMap(page -> getPageByViewMode(page, view));
}

@Override
public Mono<NewPage> findById(String pageId, Optional<AclPermission> aclPermission) {
return repository.findById(pageId, aclPermission);
}

@Override
public Flux<PageDTO> findByApplicationId(String applicationId, AclPermission permission, Boolean view) {
return findNewPagesByApplicationId(applicationId, permission).flatMap(page -> getPageByViewMode(page, view));
Expand Down Expand Up @@ -521,22 +516,22 @@ public Mono<NewPage> archive(NewPage page) {
}

@Override
public Mono<NewPage> archiveWithoutPermissionById(String id) {
return archiveByIdEx(id, Optional.empty());
public Mono<NewPage> archiveByIdWithoutPermission(String id) {
return archiveByIdEx(id, null);
}

@Override
public Mono<NewPage> archiveById(String id) {
return archiveByIdEx(id, Optional.of(pagePermission.getDeletePermission()));
return archiveByIdEx(id, pagePermission.getDeletePermission());
}

@Override
public Mono<Boolean> archiveByIds(Collection<String> idList) {
return repository.archiveAllById(idList);
}

public Mono<NewPage> archiveByIdEx(String id, Optional<AclPermission> permission) {
Mono<NewPage> pageMono = this.findById(id, permission)
private Mono<NewPage> archiveByIdEx(String id, AclPermission permission) {
Mono<NewPage> pageMono = findById(id, permission)
.switchIfEmpty(
Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE_ID, id)))
.cache();
Expand Down
9 changes: 2 additions & 7 deletions ...src/main/java/com/appsmith/server/newpages/importable/NewPageImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,15 +359,10 @@ Mono<Application> savePagesToApplicationMono(
// This does not apply to the traditional import via file approach
return Flux.fromIterable(invalidPageIds)
.flatMap(pageId -> {
return applicationPageService.deleteUnpublishedPageWithOptionalPermission(
pageId,
Optional.empty(),
Optional.empty(),
Optional.empty(),
Optional.empty());
return applicationPageService.deleteUnpublishedPage(pageId, null, null, null, null);
})
.flatMap(page -> newPageService
.archiveWithoutPermissionById(page.getId())
.archiveByIdWithoutPermission(page.getId())
.onErrorResume(e -> {
log.debug(
"Unable to archive page {} with error {}",
Expand Down
1 change: 1 addition & 0 deletions ...er/appsmith-server/src/main/java/com/appsmith/server/repositories/AppsmithRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public interface AppsmithRepository<T extends BaseDomain> {

Mono<T> findById(String id, AclPermission permission);

@Deprecated(forRemoval = true)
Mono<T> findById(String id, Optional<AclPermission> permission);

Mono<T> updateById(String id, T resource, AclPermission permission);
Expand Down
2 changes: 1 addition & 1 deletion ...erver/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public Mono<T> findById(String id, AclPermission permission) {
/**
* @deprecated using `Optional` for function arguments is an anti-pattern.
*/
@Deprecated
@Deprecated(forRemoval = true)
public Mono<T> findById(String id, Optional<AclPermission> permission) {
return findById(id, permission.orElse(null));
}
Expand Down
11 changes: 5 additions & 6 deletions ...psmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import reactor.core.publisher.Mono;

import java.util.List;
import java.util.Optional;

public interface ApplicationPageServiceCE {

Expand Down Expand Up @@ -50,12 +49,12 @@ Mono<PageDTO> getPageAndMigrateDslByBranchAndDefaultPageId(

Mono<PageDTO> deleteUnpublishedPageByBranchAndDefaultPageId(String defaultPageId, String branchName);

Mono<PageDTO> deleteUnpublishedPageWithOptionalPermission(
Mono<PageDTO> deleteUnpublishedPage(
String id,
Optional<AclPermission> deletePagePermission,
Optional<AclPermission> readApplicationPermission,
Optional<AclPermission> deleteCollectionPermission,
Optional<AclPermission> deleteActionPermission);
AclPermission deletePagePermission,
AclPermission readApplicationPermission,
AclPermission deleteCollectionPermission,
AclPermission deleteActionPermission);

Mono<PageDTO> deleteUnpublishedPage(String id);

Expand Down
Loading