Skip to content

Commit

Permalink
chore: Add empty policyMap when policies are empty or null to fix NPE (
Browse files Browse the repository at this point in the history
…#36374)

## Description
We have quite a lot of objects in the DB for which we expect the
policies should be present but that's not the case. This is causing
issues when we migrated to using `policyMap` where we didn't create
empty map if the policies object is empty expecting all such entries are
anyway not accessible to the user.
We can query mongodb to find such entries:  
```
db
	.collectionName
	.find({
		policies: [],
		deletedAt: {$exists: false}
	})
```
We expect collections like plugins, customJSlibs etc to have empty
values for policies and hence policyMap was not migrated for this, but
we are seeing the same pattern for other collections as well like
newAction, workspace etc. which is why we are seeing NPEs. With this PR
we intend to remove this NPE by adding a empty map to policyMap field.

With prod dump this is taking 340sec so we need to plan accordingly.

/test Sanity

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10903942123>
> Commit: c1573a6
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10903942123&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Tue, 17 Sep 2024 13:41:53 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Implemented a database migration to ensure all relevant documents have
a defined policy map, improving data consistency.
  
- **Bug Fixes**
- Addressed issues where documents had null or missing policy maps by
updating them to an empty policy map.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
abhvsn authored Sep 18, 2024
1 parent 74933fc commit 0388f63
Showing 1 changed file with 113 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package com.appsmith.server.migrations.db.ce;

import com.appsmith.external.helpers.Stopwatch;
import com.mongodb.client.result.UpdateResult;
import io.mongock.api.annotations.ChangeUnit;
import io.mongock.api.annotations.Execution;
import io.mongock.api.annotations.RollbackExecution;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.mongodb.core.ReactiveMongoTemplate;
import org.springframework.data.mongodb.core.query.Criteria;
import org.springframework.data.mongodb.core.query.Query;
import org.springframework.data.mongodb.core.query.Update;
import org.springframework.data.mongodb.core.query.UpdateDefinition;
import reactor.core.publisher.Mono;

import java.util.HashMap;
import java.util.Set;

import static com.appsmith.server.migrations.constants.FieldName.POLICY_MAP;
import static org.springframework.data.mongodb.core.query.Criteria.where;

@RequiredArgsConstructor
@Slf4j
@ChangeUnit(order = "062", id = "add_empty_policyMap_for_null_entries")
public class Migration062AddEmptyPolicyMapForNullValues {

private final ReactiveMongoTemplate mongoTemplate;

private static final Set<String> COLLECTION_NAMES = Set.of(
"actionCollection",
"application",
"applicationSnapshot",
"asset",
"collection",
"config",
"customJSLib",
"datasource",
"datasourceStorage",
"datasourceStorageStructure",
"emailVerificationToken",
"gitDeployKeys",
"newAction",
"newPage",
"passwordResetToken",
"permissionGroup",
"plugin",
"theme",
"user",
"userData",
"workspace",
"approvalRequest",
"auditLog",
"datasourceConfigurationStructure",
"environment",
"inviteUser",
"knowledgeStore",
"module",
"moduleInstance",
"package",
"role",
"userApiKey",
"userGroup",
"workflow");

@RollbackExecution
public void rollbackExecution() {}

@Execution
public void execute() {
Stopwatch stopwatch = new Stopwatch("Migration062AddEmptyPolicyMapForNullValues");
Mono.whenDelayError(COLLECTION_NAMES.stream()
.map(c -> addEmptyPolicyMapForNullEntries(mongoTemplate, c))
.toList())
.onErrorResume(error -> {
String errorPrefix = "Error while adding empty policyMap for null values";
// As we are using Mono.whenDelayError, we expect multiple errors to be suppressed in a single error
if (error.getSuppressed().length > 0) {
for (Throwable suppressed : error.getSuppressed()) {
log.error(errorPrefix, suppressed);
}
} else {
log.error(errorPrefix, error);
}
return Mono.error(error);
})
.block();
stopwatch.stopAndLogTimeInMillis();
}

private static Mono<Void> addEmptyPolicyMapForNullEntries(
ReactiveMongoTemplate mongoTemplate, String collectionName) {
log.info("Adding empty policyMap for empty policies {}", collectionName);

// Update policyMap for documents where policies is empty or null
Criteria policyMapNotExists =
new Criteria().orOperator(where(POLICY_MAP).exists(false));
Criteria notDeletedCriteria = where("deletedAt").isNull();

final Query query = new Query().addCriteria(policyMapNotExists).addCriteria(notDeletedCriteria);
UpdateDefinition update = new Update().set(POLICY_MAP, new HashMap<>());
final Mono<UpdateResult> convertToMap = mongoTemplate.updateMulti(query, update, collectionName);

return convertToMap
.elapsed()
.doOnSuccess(it -> log.info(
"Migrated {} documents in {} in {}ms",
it.getT2().getModifiedCount(),
collectionName,
it.getT1()))
.then();
}
}

0 comments on commit 0388f63

Please sign in to comment.