Fix add-slug migration messing up existing slugs, if any.

Migrations should be more resilient in regards to existing data.
This commit is contained in:
Shrikant Kandula 2020-03-28 11:15:50 +00:00 committed by Arpit Mohan
parent e6830ed6a4
commit 35a2722305
3 changed files with 32 additions and 13 deletions

View File

@ -34,8 +34,8 @@ public class Organization extends BaseDomain {
@Indexed(unique = true)
private String slug;
public String getSlug() {
return slug == null ? toSlug(name) : slug;
public String makeSlug() {
return toSlug(name);
}
public static String toSlug(String text) {

View File

@ -1,10 +1,11 @@
package com.appsmith.server.migrations;
import com.appsmith.server.domains.*;
import com.appsmith.server.services.OrganizationService;
import com.github.cloudyrock.mongock.ChangeLog;
import com.github.cloudyrock.mongock.ChangeSet;
import com.mongodb.DuplicateKeyException;
import lombok.extern.slf4j.Slf4j;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.data.domain.Sort;
import org.springframework.data.mongodb.UncategorizedMongoDbException;
import org.springframework.data.mongodb.core.MongoTemplate;
@ -20,6 +21,10 @@ public class DatabaseChangelog {
/**
* A private, pure utility function to create instances of Index objects to pass to `IndexOps.ensureIndex` method.
* Note: The order of the fields here is important. An index with the fields `"name", "organizationId"` is different
* from an index with the fields `"organizationId", "name"`. If an index exists with the first ordering and we try
* to **ensure** an index with the same name but the second ordering of fields, errors will show up and bad things
* WILL happen.
*/
private static Index makeIndex(String... fields) {
if (fields.length == 1) {
@ -82,11 +87,20 @@ public class DatabaseChangelog {
}
@ChangeSet(order = "003", id = "add-org-slugs", author = "")
public void addOrgSlugs(MongoTemplate mongoTemplate) {
public void addOrgSlugs(MongoTemplate mongoTemplate, OrganizationService organizationService) {
// For all existing organizations, add a slug field, which should be unique.
// We are blocking here for adding a slug to each existing organization. This is bad and slow. Do NOT copy this
// code fragment into the services' control flow. This is a single migration code and is expected to run once in
// lifetime of a deployment.
for (Organization organization : mongoTemplate.findAll(Organization.class)) {
organization.setSlug(organization.getSlug());
mongoTemplate.save(organization);
if (organization.getSlug() == null) {
organizationService.getNextUniqueSlug(organization.makeSlug())
.doOnSuccess(slug -> {
organization.setSlug(slug);
mongoTemplate.save(organization);
})
.block();
}
}
}
@ -106,7 +120,7 @@ public class DatabaseChangelog {
ensureIndexes(mongoTemplate, Application.class,
createdAtIndex,
makeIndex("name", "organizationId").unique().named("organization_application_compound_index")
makeIndex("organizationId", "name").unique().named("organization_application_compound_index")
);
ensureIndexes(mongoTemplate, Collection.class,
@ -120,7 +134,7 @@ public class DatabaseChangelog {
ensureIndexes(mongoTemplate, Datasource.class,
createdAtIndex,
makeIndex("name", "organizationId").unique().named("organization_datasource_compound_index")
makeIndex("organizationId", "name").unique().named("organization_datasource_compound_index")
);
ensureIndexes(mongoTemplate, InviteUser.class,

View File

@ -101,11 +101,16 @@ public class OrganizationServiceImpl extends BaseService<OrganizationRepository,
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ORGANIZATION));
}
Mono<Organization> setSlugMono = getNextUniqueSlug(organization.getSlug())
.map(slug -> {
organization.setSlug(slug);
return organization;
});
Mono<Organization> setSlugMono;
if (organization.getName() == null) {
setSlugMono = Mono.just(organization);
} else {
setSlugMono = getNextUniqueSlug(organization.makeSlug())
.map(slug -> {
organization.setSlug(slug);
return organization;
});
}
Mono<Organization> organizationMono = setSlugMono
.flatMap(this::validateObject)