From 8082e320b90d2211de9a0d971b1c2620f7540d58 Mon Sep 17 00:00:00 2001
From: Andrei Bordak <Andrei_Bordak@epam.com>
Date: Tue, 26 Nov 2024 19:14:42 +0400
Subject: [PATCH] MODLD-601: Review fixes

---
 .../linked/data/service/SettingsService.java  |  5 ---
 .../data/service/SettingsServiceImpl.java     | 13 ++++---
 .../data/service/SettingsServiceImplTest.java | 36 +++++--------------
 3 files changed, 15 insertions(+), 39 deletions(-)

diff --git a/src/main/java/org/folio/linked/data/service/SettingsService.java b/src/main/java/org/folio/linked/data/service/SettingsService.java
index 52a4b92c..e101e8c9 100644
--- a/src/main/java/org/folio/linked/data/service/SettingsService.java
+++ b/src/main/java/org/folio/linked/data/service/SettingsService.java
@@ -1,11 +1,6 @@
 package org.folio.linked.data.service;
 
-import java.util.Optional;
-import org.folio.linked.data.domain.dto.SettingsItem;
-
 public interface SettingsService {
 
-  Optional<SettingsItem> getSetting(String scope, String key);
-
   boolean isSettingEnabled(String scope, String key, String property);
 }
diff --git a/src/main/java/org/folio/linked/data/service/SettingsServiceImpl.java b/src/main/java/org/folio/linked/data/service/SettingsServiceImpl.java
index 3456a50e..1ad999c7 100644
--- a/src/main/java/org/folio/linked/data/service/SettingsServiceImpl.java
+++ b/src/main/java/org/folio/linked/data/service/SettingsServiceImpl.java
@@ -18,13 +18,6 @@ public class SettingsServiceImpl implements SettingsService {
 
   private final SettingsClient settingsClient;
 
-  @Override
-  public Optional<SettingsItem> getSetting(String scope, String key) {
-    return ofNullable(settingsClient.getEntries(buildQuery(scope, key)))
-      .map(ResponseEntity::getBody)
-      .flatMap(body -> body.getItems().stream().findFirst());
-  }
-
   @Override
   public boolean isSettingEnabled(String scope, String key, String property) {
     return getSetting(scope, key)
@@ -32,6 +25,12 @@ public boolean isSettingEnabled(String scope, String key, String property) {
       .orElse(false);
   }
 
+  private Optional<SettingsItem> getSetting(String scope, String key) {
+    return ofNullable(settingsClient.getEntries(buildQuery(scope, key)))
+      .map(ResponseEntity::getBody)
+      .flatMap(body -> body.getItems().stream().findFirst());
+  }
+
   private boolean isPropertyEnabled(SettingsItem item, String property) {
     var propertyValue = item.getValue().getOrDefault(property, false);
     return propertyValue instanceof Boolean value && value;
diff --git a/src/test/java/org/folio/linked/data/service/SettingsServiceImplTest.java b/src/test/java/org/folio/linked/data/service/SettingsServiceImplTest.java
index a4d10655..573cb69a 100644
--- a/src/test/java/org/folio/linked/data/service/SettingsServiceImplTest.java
+++ b/src/test/java/org/folio/linked/data/service/SettingsServiceImplTest.java
@@ -1,13 +1,11 @@
 package org.folio.linked.data.service;
 
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import java.util.Map;
 import java.util.stream.Stream;
 import org.folio.linked.data.client.SettingsClient;
 import org.folio.linked.data.domain.dto.SettingsItem;
@@ -35,33 +33,16 @@ class SettingsServiceImplTest {
   private SettingsClient settingsClient;
 
   @Test
-  void getSettings_shouldReturnSettingsItem() {
+  void isSettingEnabled_shouldReturnFalse_ifSettingsAreEmpty() {
     // given
-    var settingsItem = new SettingsItem().value(Map.of("property", true));
-    when(settingsClient.getEntries("(scope==scope and key==key)"))
-      .thenReturn(new ResponseEntity<>(
-        new SettingsSearchResponse().addItemsItem(settingsItem), HttpStatus.OK));
-
-    // when
-    var setting = settingsService.getSetting("scope", "key");
-
-    // then
-    assertThat(setting)
-      .get()
-      .satisfies(actual -> assertEquals(actual, settingsItem));
-    verify(settingsClient).getEntries("(scope==scope and key==key)");
-  }
-
-  @Test
-  void getSettings_shouldReturnEmptySettingsItem() {
-    // given
-    when(settingsClient.getEntries(any())).thenReturn(null);
+    when(settingsClient.getEntries(any()))
+      .thenReturn(new ResponseEntity<>(new SettingsSearchResponse(), HttpStatus.OK));
 
     // when
-    var setting = settingsService.getSetting("scope", "key");
+    boolean isEnabled = settingsService.isSettingEnabled("scope", "key", "property");
 
     // then
-    assertThat(setting).isEmpty();
+    assertFalse(isEnabled);
   }
 
   @ParameterizedTest
@@ -73,16 +54,17 @@ void isSettingEnabled_shouldBaseOnSettingPropertyValue(SettingsItem item, boolea
         new SettingsSearchResponse().addItemsItem(item), HttpStatus.OK));
 
     // when
-    boolean settingEnabled = settingsService.isSettingEnabled("scope", "key", "property");
+    boolean isEnabled = settingsService.isSettingEnabled("scope", "key", "property");
 
     // then
-    assertEquals(expected, settingEnabled);
+    assertEquals(expected, isEnabled);
   }
 
   private static Stream<Arguments> settingsProvider() {
     return Stream.of(
       arguments(new SettingsItem().putValueItem("property", true), true),
       arguments(new SettingsItem().putValueItem("property", false), false),
+      arguments(new SettingsItem().putValueItem("property", new Object()), false),
       arguments(new SettingsItem(), false)
     );
   }