From c615adafa53021f098c507084474ec4913bbea43 Mon Sep 17 00:00:00 2001 From: Arpit Mohan Date: Thu, 12 Nov 2020 19:01:40 +0530 Subject: [PATCH 1/5] Fixing the telemetry on self-hosted instances (#1714) --- app/client/src/configs/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/client/src/configs/index.ts b/app/client/src/configs/index.ts index bae62dc920..e1dd4cea58 100644 --- a/app/client/src/configs/index.ts +++ b/app/client/src/configs/index.ts @@ -178,6 +178,9 @@ export const getAppsmithConfigs = (): AppsmithUIConfigs => { APPSMITH_FEATURE_CONFIGS.segment.ceKey, ); + // We enable segment tracking if either the Cloud API key is set or the self-hosted CE key is set + segment.enabled = segment.enabled || segmentCEKey.enabled; + let sentryTelemetry = true; // Turn off all analytics if telemetry is disabled if (APPSMITH_FEATURE_CONFIGS.disableTelemetry) { From be1bc0d27fc2077223a7afcaa31aef93665b4eb4 Mon Sep 17 00:00:00 2001 From: Abhinav Jha Date: Thu, 12 Nov 2020 18:58:12 +0530 Subject: [PATCH 2/5] Fix issue where creating a new application twice does not work (#1713) --- app/client/src/pages/Applications/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/client/src/pages/Applications/index.tsx b/app/client/src/pages/Applications/index.tsx index 665de33701..9c084f85ac 100644 --- a/app/client/src/pages/Applications/index.tsx +++ b/app/client/src/pages/Applications/index.tsx @@ -625,7 +625,9 @@ const ApplicationsSection = (props: any) => { { if ( - Object.entries(creatingApplicationMap).length === 0 + Object.entries(creatingApplicationMap).length === 0 || + (creatingApplicationMap && + !creatingApplicationMap[organization.id]) ) { createNewApplication( getNextEntityName( From 2b8539d651ccc4332042a11f1f7eb3816b9eb40f Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Mon, 16 Nov 2020 18:59:04 +0530 Subject: [PATCH 3/5] Adding index for unpublishedAction.pageId to bring down the number of documents fetched in mongo db query during update layouts' fetching actions on page load. (#1739) --- .../appsmith/server/migrations/DatabaseChangelog.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index d78ce1f328..6ff19b9c4e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -1215,4 +1215,15 @@ public class DatabaseChangelog { } + @ChangeSet(order = "041", id = "new-action-add-index-pageId", author = "") + public void addNewActionIndexForPageId(MongoTemplate mongoTemplate) { + + dropIndexIfExists(mongoTemplate, NewAction.class, "applicationId_deleted_createdAt_compound_index"); + + ensureIndexes(mongoTemplate, NewAction.class, + makeIndex("applicationId", "deleted", "unpublishedAction.pageId") + .named("applicationId_deleted_unpublishedPageId_compound_index") + ); + } + } From 3e1c0a731d1732c076277ce1a66782ddbf80357c Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Tue, 17 Nov 2020 12:47:49 +0530 Subject: [PATCH 4/5] Added SLA bucket for metric evaluation for 1 second (#1740) --- .../appsmith-server/src/main/resources/application.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/app/server/appsmith-server/src/main/resources/application.properties b/app/server/appsmith-server/src/main/resources/application.properties index e246eac30c..da7fe464ad 100644 --- a/app/server/appsmith-server/src/main/resources/application.properties +++ b/app/server/appsmith-server/src/main/resources/application.properties @@ -74,3 +74,4 @@ management.metrics.export.prometheus.descriptions=true management.metrics.web.server.request.ignore-trailing-slash=true management.metrics.web.server.request.autotime.percentiles=0.5, 0.9, 0.95, 0.99 management.metrics.web.server.request.autotime.percentiles-histogram=true +management.metrics.distribution.sla.[http.server.requests]=1s \ No newline at end of file From 2489754abea466124597b6c96d837c1fa077749b Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Wed, 18 Nov 2020 00:31:39 +0530 Subject: [PATCH 5/5] Email sending is now non-blocking. The blocking code for email sending is triggered and then immediately returned. (#1762) --- .../server/controllers/UserController.java | 4 +-- .../server/notifications/EmailSender.java | 31 +++++++++++-------- .../services/UserOrganizationServiceImpl.java | 8 ++--- .../appsmith/server/services/UserService.java | 2 +- .../server/services/UserServiceImpl.java | 20 ++++-------- .../services/OrganizationServiceTest.java | 8 ++--- .../server/services/UserServiceTest.java | 2 +- .../ShareOrganizationPermissionTests.java | 4 +-- 8 files changed, 36 insertions(+), 43 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java index a632354ec0..1034006549 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/UserController.java @@ -133,8 +133,8 @@ public class UserController extends BaseController { * @return List of new users who have been created/existing users who have been added to the organization. */ @PostMapping("/invite") - public Mono>> inviteUser(@RequestBody InviteUsersDTO inviteUsersDTO, @RequestHeader("Origin") String originHeader) { - return service.inviteUser(inviteUsersDTO, originHeader).collectList() + public Mono>> inviteUsers(@RequestBody InviteUsersDTO inviteUsersDTO, @RequestHeader("Origin") String originHeader) { + return service.inviteUsers(inviteUsersDTO, originHeader).collectList() .map(users -> new ResponseDTO<>(HttpStatus.OK.value(), users, null)); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java index 6c17e6bf2b..e9062dd637 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/notifications/EmailSender.java @@ -43,25 +43,30 @@ public class EmailSender { REPLY_TO = makeReplyTo(); } - public Mono sendMail(String to, String subject, String text, Map params) { - return Mono - .fromSupplier(() -> { + public Mono sendMail(String to, String subject, String text, Map params) { + + /** + * Creating a publisher which sends email in a blocking fashion, subscribing on the bounded elastic + * scheduler and then subscribing to it so that the publisher starts emitting immediately. We do not + * wait for the blocking call of `sendMailSync` to finish. BoundedElastic scheduler would ensure that + * when the number of tasks go beyond the number of available threads, the tasks would be deferred till + * a thread becomes available without overloading the server. + */ + Mono.fromCallable(() -> { try { return replaceEmailTemplate(text, params); } catch (IOException e) { throw Exceptions.propagate(e); } }) - // Sending email is a high cost I/O operation. Schedule the same on non-netty threads - // to implement a fire-and-forget strategy. - // CAUTION : We may run into scenarios where too many tasks have been created and queued and the master tasks have already exited with success. - .doOnNext(emailBody -> - Mono.fromRunnable(() -> sendMailSync(to, subject, emailBody)) - // Scheduling using boundedElastic because the number of active tasks are capped - // and hence not allowing the background threads to grow indefinitely - .subscribeOn(Schedulers.boundedElastic()) - .subscribe() - ); + .doOnNext(emailBody -> { + sendMailSync(to, subject, emailBody); + }) + .subscribeOn(Schedulers.boundedElastic()) + .subscribe(); + + // Creating a hot source which would be created, emitted, and returned immediately. + return Mono.just(Boolean.TRUE); } /** diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java index 583c60a34a..2964ccdb11 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java @@ -300,15 +300,11 @@ public class UserOrganizationServiceImpl implements UserOrganizationService { params.put("inviteUrl", originHeader); params.put("user_role_name", userRole.getRoleName()); - Mono emailMono = emailSender.sendMail(user.getEmail(), + Mono emailMono = emailSender.sendMail(user.getEmail(), "Appsmith: Your Role has been changed", UPDATE_ROLE_EXISTING_USER_TEMPLATE, params); return emailMono - .thenReturn(addedOrganization) - .onErrorResume(error -> { - log.error("Unable to send role change email to {}. Cause: ", user.getEmail(), error); - return Mono.just(addedOrganization); - }); + .thenReturn(addedOrganization); }); } else { // If the roleName was not present, then it implies that the user is being removed from the org. diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java index 0e5085e31f..d6a141f524 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserService.java @@ -29,5 +29,5 @@ public interface UserService extends CrudService { Mono userCreate(User user); - Flux inviteUser(InviteUsersDTO inviteUsersDTO, String originHeader); + Flux inviteUsers(InviteUsersDTO inviteUsersDTO, String originHeader); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index 7d777f45e2..32130dc0bf 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -475,7 +475,7 @@ public class UserServiceImpl extends BaseService i * platform. This flow also ensures that a default organization name is created for the user. The new user is then * given admin permissions to the default organization. *

- * For new user invite flow, please {@link UserService#inviteUser(InviteUsersDTO, String)} + * For new user invite flow, please {@link UserService#inviteUsers(InviteUsersDTO, String)} * * @param user User object representing the user to be created/enabled. * @return Publishes the user object, after having been saved. @@ -568,7 +568,7 @@ public class UserServiceImpl extends BaseService i * @return Publishes the invited users, after being saved with the new organization ID. */ @Override - public Flux inviteUser(InviteUsersDTO inviteUsersDTO, String originHeader) { + public Flux inviteUsers(InviteUsersDTO inviteUsersDTO, String originHeader) { if (originHeader == null || originHeader.isBlank()) { return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORIGIN)); @@ -633,16 +633,12 @@ public class UserServiceImpl extends BaseService i log.debug("Going to send email to user {} informing that the user has been added to new organization {}", existingUser.getEmail(), organization.getName()); params.put("inviteUrl", originHeader); - Mono emailMono = emailSender.sendMail(existingUser.getEmail(), + Mono emailMono = emailSender.sendMail(existingUser.getEmail(), "Appsmith: You have been added to a new organization", USER_ADDED_TO_ORGANIZATION_EMAIL_TEMPLATE, params); return emailMono - .thenReturn(existingUser) - .onErrorResume(error -> { - log.error("Unable to send invite user email to {}. Cause: ", existingUser.getEmail(), error); - return Mono.just(existingUser); - }); + .thenReturn(existingUser); }) .switchIfEmpty(createNewUserAndSendInviteEmail(username, originHeader, params)); }) @@ -705,15 +701,11 @@ public class UserServiceImpl extends BaseService i ); params.put("inviteUrl", inviteUrl); - Mono emailMono = emailSender.sendMail(createdUser.getEmail(), "Invite for Appsmith", INVITE_USER_EMAIL_TEMPLATE, params); + Mono emailMono = emailSender.sendMail(createdUser.getEmail(), "Invite for Appsmith", INVITE_USER_EMAIL_TEMPLATE, params); // We have sent out the emails. Just send back the saved user. return emailMono - .thenReturn(createdUser) - .onErrorResume(error -> { - log.error("Unable to send invite user email to {}. Cause: ", createdUser.getEmail(), error); - return Mono.just(createdUser); - }); + .thenReturn(createdUser); }); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java index b65eef55e8..06d85165ed 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/OrganizationServiceTest.java @@ -335,7 +335,7 @@ public class OrganizationServiceTest { inviteUsersDTO.setOrgId(organization1.getId()); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_ADMIN.getName()); - return userService.inviteUser(inviteUsersDTO, "http://localhost:8080") + return userService.inviteUsers(inviteUsersDTO, "http://localhost:8080") .collectList(); }) .cache(); @@ -395,7 +395,7 @@ public class OrganizationServiceTest { inviteUsersDTO.setOrgId(organization1.getId()); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_ADMIN.getName()); - return userService.inviteUser(inviteUsersDTO, "http://localhost:8080") + return userService.inviteUsers(inviteUsersDTO, "http://localhost:8080") .collectList(); }) .cache(); @@ -468,7 +468,7 @@ public class OrganizationServiceTest { inviteUsersDTO.setOrgId(organization1.getId()); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_VIEWER.getName()); - return userService.inviteUser(inviteUsersDTO, "http://localhost:8080") + return userService.inviteUsers(inviteUsersDTO, "http://localhost:8080") .collectList(); }) .cache(); @@ -857,7 +857,7 @@ public class OrganizationServiceTest { inviteUsersDTO.setOrgId(organization1.getId()); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_VIEWER.getName()); - return userService.inviteUser(inviteUsersDTO, "http://localhost:8080") + return userService.inviteUsers(inviteUsersDTO, "http://localhost:8080") .collectList(); }) .cache(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java index bd5819ca4e..52069223b9 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserServiceTest.java @@ -361,7 +361,7 @@ public class UserServiceTest { inviteUsersDTO.setOrgId(organization1.getId()); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_VIEWER.getName()); - return userService.inviteUser(inviteUsersDTO, "http://localhost:8080") + return userService.inviteUsers(inviteUsersDTO, "http://localhost:8080") .collectList(); }).block(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java index 31731ce2da..68a7582954 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ShareOrganizationPermissionTests.java @@ -75,7 +75,7 @@ public class ShareOrganizationPermissionTests { emails.add("admin@solutiontest.com"); inviteUsersDTO.setUsernames(emails); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_ADMIN.getName()); - userService.inviteUser(inviteUsersDTO, "http://localhost:8080").blockLast(); + userService.inviteUsers(inviteUsersDTO, "http://localhost:8080").blockLast(); emails.clear(); @@ -83,7 +83,7 @@ public class ShareOrganizationPermissionTests { emails.add("developer@solutiontest.com"); inviteUsersDTO.setUsernames(emails); inviteUsersDTO.setRoleName(AppsmithRole.ORGANIZATION_DEVELOPER.getName()); - userService.inviteUser(inviteUsersDTO, "http://localhost:8080").blockLast(); + userService.inviteUsers(inviteUsersDTO, "http://localhost:8080").blockLast(); } @Test