Skip to content

Commit

Permalink
Revert accidental "improve fallback to remote dc validation" (#1877)
Browse files Browse the repository at this point in the history
This reverts commit 5cc4fe1.
  • Loading branch information
moscicky committed Jul 3, 2024
1 parent 5cc4fe1 commit 19beb58
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import pl.allegro.tech.hermes.api.RetentionTime;
import pl.allegro.tech.hermes.api.Topic;
import pl.allegro.tech.hermes.management.api.validator.ApiPreconditions;
import pl.allegro.tech.hermes.management.config.TopicProperties;
import pl.allegro.tech.hermes.management.domain.auth.RequestUser;
import pl.allegro.tech.hermes.management.domain.owner.validator.OwnerIdValidator;
import pl.allegro.tech.hermes.management.domain.topic.CreatorRights;
Expand All @@ -28,21 +27,18 @@ public class TopicValidator {
private final TopicLabelsValidator topicLabelsValidator;
private final SchemaRepository schemaRepository;
private final ApiPreconditions apiPreconditions;
private final TopicProperties topicProperties;

@Autowired
public TopicValidator(OwnerIdValidator ownerIdValidator,
ContentTypeValidator contentTypeValidator,
TopicLabelsValidator topicLabelsValidator,
SchemaRepository schemaRepository,
ApiPreconditions apiPreconditions,
TopicProperties topicProperties) {
ApiPreconditions apiPreconditions) {
this.ownerIdValidator = ownerIdValidator;
this.contentTypeValidator = contentTypeValidator;
this.topicLabelsValidator = topicLabelsValidator;
this.schemaRepository = schemaRepository;
this.apiPreconditions = apiPreconditions;
this.topicProperties = topicProperties;
}

public void ensureCreatedTopicIsValid(Topic created, RequestUser createdBy, CreatorRights creatorRights) {
Expand All @@ -51,8 +47,8 @@ public void ensureCreatedTopicIsValid(Topic created, RequestUser createdBy, Crea
checkContentType(created);
checkTopicLabels(created);

if ((created.isFallbackToRemoteDatacenterEnabled() != topicProperties.isDefaultFallbackToRemoteDatacenterEnabled()) && !createdBy.isAdmin()) {
throw new TopicValidationException("User is not allowed to set non-default fallback to remote datacenter for this topic");
if (created.isFallbackToRemoteDatacenterEnabled() && !createdBy.isAdmin()) {
throw new TopicValidationException("User is not allowed to enable fallback to remote datacenter");
}

if (created.getChaos().mode() != ChaosMode.DISABLED && !createdBy.isAdmin()) {
Expand All @@ -76,8 +72,8 @@ public void ensureUpdatedTopicIsValid(Topic updated, Topic previous, RequestUser
checkOwner(updated);
checkTopicLabels(updated);

if ((previous.isFallbackToRemoteDatacenterEnabled() != updated.isFallbackToRemoteDatacenterEnabled()) && !modifiedBy.isAdmin()) {
throw new TopicValidationException("User is not allowed to update fallback to remote datacenter for this topic");
if (!previous.isFallbackToRemoteDatacenterEnabled() && updated.isFallbackToRemoteDatacenterEnabled() && !modifiedBy.isAdmin()) {
throw new TopicValidationException("User is not allowed to enable fallback to remote datacenter");
}

if (!previous.getChaos().equals(updated.getChaos()) && !modifiedBy.isAdmin()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ public void shouldCreateTopicEvenIfExistsInBrokers() {
}

@Test
public void shouldNotAllowNonAdminUserCreateTopicWithNonDefaultFallbackToRemoteDatacenter() {
public void shouldNotAllowNonAdminUserCreateTopicWithFallbackToRemoteDatacenterEnabled() {
// given
TestSecurityProvider.setUserIsAdmin(false);
TopicWithSchema topic = topicWithSchema(
Expand All @@ -619,11 +619,11 @@ public void shouldNotAllowNonAdminUserCreateTopicWithNonDefaultFallbackToRemoteD
//then
response.expectStatus().isBadRequest();
assertThat(response.expectBody(String.class).returnResult().getResponseBody())
.contains("User is not allowed to set non-default fallback to remote datacenter");
.contains("User is not allowed to enable fallback to remote datacenter");
}

@Test
public void shouldAllowAdminUserCreateTopicWithNonDefaultFallbackToRemoteDatacenterEnabled() {
public void shouldAllowAdminUserCreateTopicWithFallbackToRemoteDatacenterEnabled() {
// given
TestSecurityProvider.setUserIsAdmin(true);
TopicWithSchema topic = topicWithSchema(
Expand All @@ -641,30 +641,27 @@ public void shouldAllowAdminUserCreateTopicWithNonDefaultFallbackToRemoteDatacen
}

@Test
public void shouldNotAllowNonAdminUserToChangeFallbackToRemoteDatacenter() {
public void shouldNotAllowNonAdminUserToEnableFallbackToRemoteDatacenter() {
// given
Topic topic = hermes.initHelper().createTopic(topicWithRandomName()
.withFallbackToRemoteDatacenterEnabled().build());
Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build());
TestSecurityProvider.setUserIsAdmin(false);
PatchData patchData = PatchData.from(ImmutableMap.of("fallbackToRemoteDatacenterEnabled", false));
PatchData patchData = PatchData.from(ImmutableMap.of("fallbackToRemoteDatacenterEnabled", true));

// when
WebTestClient.ResponseSpec response = hermes.api().updateTopic(topic.getQualifiedName(), patchData);

//then
response.expectStatus().isBadRequest();
assertThat(response.expectBody(String.class).returnResult().getResponseBody())
.contains("User is not allowed to update fallback to remote datacenter for this topic");
.contains("User is not allowed to enable fallback to remote datacenter");
}

@Test
public void shouldAllowAdminUserToChangeFallbackToRemoteDatacenter() {
public void shouldAllowAdminUserToEnableFallbackToRemoteDatacenter() {
// given
Topic topic = hermes.initHelper().createTopic(topicWithRandomName()
.withFallbackToRemoteDatacenterEnabled()
.build());
Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build());
TestSecurityProvider.setUserIsAdmin(true);
PatchData patchData = PatchData.from(ImmutableMap.of("fallbackToRemoteDatacenterEnabled", false));
PatchData patchData = PatchData.from(ImmutableMap.of("fallbackToRemoteDatacenterEnabled", true));

// when
WebTestClient.ResponseSpec response = hermes.api().updateTopic(topic.getQualifiedName(), patchData);
Expand Down

0 comments on commit 19beb58

Please sign in to comment.