MODFQMMGR-331: Retry materialized view refreshes on failure (#295)

folio-org · Aug 1, 2024 · 64c9fb1 · 64c9fb1
1 parent 8c8a4cd
commit 64c9fb1
Show file tree

Hide file tree

Showing 10 changed files with 189 additions and 48 deletions.
diff --git a/src/main/java/org/folio/fqm/repository/DataRefreshRepository.java b/src/main/java/org/folio/fqm/repository/DataRefreshRepository.java
@@ -28,24 +28,17 @@ public class DataRefreshRepository {
 
   public static final Field<String> CURRENCY_FIELD = field("currency", String.class);
   public static final Field<Double> EXCHANGE_RATE_FIELD = field("exchange_rate", Double.class);
+  public static final String EXCHANGE_RATE_TABLE = "currency_exchange_rates";
 
-  private static final String REFRESH_MATERIALIZED_VIEW_SQL = "REFRESH MATERIALIZED VIEW CONCURRENTLY ";
+  private static final String REFRESH_MATERIALIZED_VIEW_CONCURRENTLY_SQL = "REFRESH MATERIALIZED VIEW CONCURRENTLY ";
+  private static final String REFRESH_MATERIALIZED_VIEW_SQL = "REFRESH MATERIALIZED VIEW ";
   private static final String GET_EXCHANGE_RATE_PATH = "finance/exchange-rate";
   private static final String GET_LOCALE_SETTINGS_PATH = "configurations/entries";
   private static final Map<String, String> GET_LOCALE_SETTINGS_PARAMS = Map.of(
     "query", "(module==ORG and configName==localeSettings)"
   );
 
-  private static final List<String> MATERIALIZED_VIEW_NAMES = List.of(
-    "drv_circulation_loan_status",
-    "drv_inventory_item_status",
-    "drv_pol_payment_status",
-    "drv_pol_receipt_status",
-    "drv_inventory_statistical_code_full",
-    "drv_languages"
-  );
-
-  static final List<String> SYSTEM_SUPPORTED_CURRENCIES = List.of(
+  private static final List<String> SYSTEM_SUPPORTED_CURRENCIES = List.of(
     "USD",
     "EUR",
     "JPY",
@@ -85,21 +78,47 @@ public class DataRefreshRepository {
 
   private final SimpleHttpClient simpleHttpClient;
 
-  public void refreshMaterializedViews(String tenantId) {
-    for (String matViewName : MATERIALIZED_VIEW_NAMES) {
+  /**
+   * Refresh a list of materialized views
+   *
+   * @param tenantId              Tenant ID
+   * @param viewsToRefresh        List of materialized views to refresh
+   * @param refreshConcurrently   Whether to execute a concurrent refresh
+   * @return                      List of all materialized views that failed to refresh
+   */
+  public List<String> refreshMaterializedViews(String tenantId, List<String> viewsToRefresh, boolean refreshConcurrently) {
+    List<String> failedRefreshes = new ArrayList<>();
+    String refreshType = refreshConcurrently ? "concurrently" : "non-concurrently";
+    for (String matViewName : viewsToRefresh) {
       String fullName = tenantId + "_mod_fqm_manager." + matViewName;
-      log.info("Refreshing materialized view {}", fullName);
-      jooqContext.execute(REFRESH_MATERIALIZED_VIEW_SQL + fullName);
+      log.info("Refreshing materialized view {} {}", fullName, refreshType);
+      try {
+        if (refreshConcurrently) {
+          jooqContext.execute(REFRESH_MATERIALIZED_VIEW_CONCURRENTLY_SQL + fullName);
+        } else {
+          jooqContext.execute(REFRESH_MATERIALIZED_VIEW_SQL + fullName);
+        }
+      } catch (Exception e) {
+        log.info("Failed to refresh materialized view {} {}.", matViewName, refreshType);
+        failedRefreshes.add(matViewName);
+      }
     }
+    return failedRefreshes;
   }
 
-  public void refreshExchangeRates(String tenantId) {
+  /**
+   * Refresh the currency exchange rates for a tenant, based on the tenant's default system currency.
+   *
+   * @param tenantId    Tenant ID
+   * @return            True if refresh successful, false otherwise
+   */
+  public boolean refreshExchangeRates(String tenantId) {
     log.info("Refreshing exchange rates");
-    String fullTableName = tenantId + "_mod_fqm_manager.currency_exchange_rates";
+    String fullTableName = tenantId + "_mod_fqm_manager." + EXCHANGE_RATE_TABLE;
     String systemCurrency = getSystemCurrencyCode();
     if (!SYSTEM_SUPPORTED_CURRENCIES.contains(systemCurrency)) {
       log.info("System currency does not support automatic exchange rate calculation");
-      return;
+      return false;
     }
 
     List<Record2<String, Double>> exchangeRates = new ArrayList<>();
@@ -120,6 +139,7 @@ public void refreshExchangeRates(String tenantId) {
       .doUpdate()
       .set(EXCHANGE_RATE_FIELD, DSL.field("EXCLUDED." + EXCHANGE_RATE_FIELD.getName(), Double.class))
       .execute();
+    return true;
   }
 
   private String getSystemCurrencyCode() {

diff --git a/src/main/java/org/folio/fqm/resource/DataRefreshController.java b/src/main/java/org/folio/fqm/resource/DataRefreshController.java
@@ -1,9 +1,9 @@
 package org.folio.fqm.resource;
 
 import lombok.RequiredArgsConstructor;
+import org.folio.fqm.domain.dto.DataRefreshResponse;
 import org.folio.fqm.service.DataRefreshService;
 import org.folio.spring.FolioExecutionContext;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.RestController;
 
@@ -14,8 +14,7 @@ public class DataRefreshController implements MaterializedViewsApi {
   private final DataRefreshService dataRefreshService;
 
   @Override
-  public ResponseEntity<Void> refreshData() {
-    dataRefreshService.refreshData(executionContext.getTenantId());
-    return new ResponseEntity<>(HttpStatus.NO_CONTENT);
+  public ResponseEntity<DataRefreshResponse> refreshData() {
+    return ResponseEntity.ok(dataRefreshService.refreshData(executionContext.getTenantId()));
   }
 }
diff --git a/src/main/java/org/folio/fqm/service/DataRefreshService.java b/src/main/java/org/folio/fqm/service/DataRefreshService.java
@@ -1,16 +1,43 @@
 package org.folio.fqm.service;
 
 import lombok.RequiredArgsConstructor;
+import org.folio.fqm.domain.dto.DataRefreshResponse;
 import org.folio.fqm.repository.DataRefreshRepository;
 import org.springframework.stereotype.Service;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.folio.fqm.repository.DataRefreshRepository.EXCHANGE_RATE_TABLE;
+
 @Service
 @RequiredArgsConstructor
 public class DataRefreshService {
   private final DataRefreshRepository dataRefreshRepository;
 
-  public void refreshData(String tenantId) {
-    dataRefreshRepository.refreshMaterializedViews(tenantId);
-    dataRefreshRepository.refreshExchangeRates(tenantId);
+  static final List<String> MATERIALIZED_VIEW_NAMES = List.of(
+    "drv_circulation_loan_status",
+    "drv_inventory_item_status",
+    "drv_pol_payment_status",
+    "drv_pol_receipt_status",
+    "drv_inventory_statistical_code_full",
+    "drv_languages"
+  );
+
+  public DataRefreshResponse refreshData(String tenantId) {
+    List<String> failedConcurrentRefreshes = dataRefreshRepository.refreshMaterializedViews(tenantId, MATERIALIZED_VIEW_NAMES, true);
+    List<String> failedRefreshes = dataRefreshRepository.refreshMaterializedViews(tenantId, failedConcurrentRefreshes, false);
+    List<String> successRefreshes = new ArrayList<>(MATERIALIZED_VIEW_NAMES
+      .stream()
+      .filter(matView -> !failedRefreshes.contains(matView))
+      .toList());
+    if (dataRefreshRepository.refreshExchangeRates(tenantId)) {
+      successRefreshes.add(EXCHANGE_RATE_TABLE);
+    } else {
+      failedRefreshes.add(EXCHANGE_RATE_TABLE);
+    }
+    return new DataRefreshResponse()
+      .successfulRefresh(successRefreshes)
+      .failedRefresh(failedRefreshes);
   }
 }
diff --git a/src/main/resources/swagger.api/mod-fqm-manager.yaml b/src/main/resources/swagger.api/mod-fqm-manager.yaml
@@ -51,8 +51,12 @@ paths:
         - materializedViews
       description: Refresh all materialized views and similar data for a tenant.
       responses:
-        '204':
+        '200':
           description: 'Data refreshed'
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/dataRefreshResponse'
         '400':
           $ref: '#/components/responses/badRequestResponse'
         '500':
@@ -98,6 +102,17 @@ components:
       $ref: schemas/EntityTypeSummaryDTO.json
     purgedQueries:
       $ref: schemas/PurgedQueries.json
+    dataRefreshResponse:
+      type: object
+      properties:
+        successfulRefresh:
+          type: array
+          items:
+            type: string
+        failedRefresh:
+          type: array
+          items:
+            type: string
 
   responses:
     badRequestResponse:

diff --git a/src/test/java/org/folio/fqm/controller/DataRefreshControllerTest.java b/src/test/java/org/folio/fqm/controller/DataRefreshControllerTest.java
@@ -1,5 +1,6 @@
 package org.folio.fqm.controller;
 
+import org.folio.fqm.domain.dto.DataRefreshResponse;
 import org.folio.fqm.resource.DataRefreshController;
 import org.folio.fqm.service.DataRefreshService;
 import org.folio.spring.FolioExecutionContext;
@@ -12,7 +13,8 @@
 import org.springframework.test.web.servlet.RequestBuilder;
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
 
-import static org.mockito.Mockito.doNothing;
+import java.util.List;
+
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -31,13 +33,16 @@ class DataRefreshControllerTest {
   @Test
   void refreshDataTest() throws Exception {
     String tenantId = "tenant_01";
+    DataRefreshResponse expectedResponse = new DataRefreshResponse()
+      .successfulRefresh(List.of())
+      .failedRefresh(List.of());
     RequestBuilder requestBuilder = MockMvcRequestBuilders.post("/entity-types/materialized-views/refresh")
       .header(XOkapiHeaders.TENANT, tenantId)
       .contentType(APPLICATION_JSON);
     when(executionContext.getTenantId()).thenReturn(tenantId);
-    doNothing().when(dataRefreshService).refreshData(tenantId);
+    when(dataRefreshService.refreshData(tenantId)).thenReturn(expectedResponse);
     mockMvc.perform(requestBuilder)
-      .andExpect(status().isNoContent());
+      .andExpect(status().isOk());
     verify(dataRefreshService, times(1)).refreshData(tenantId);
   }
 }
diff --git a/src/test/java/org/folio/fqm/controller/EntityTypeControllerTest.java b/src/test/java/org/folio/fqm/controller/EntityTypeControllerTest.java
@@ -28,13 +28,13 @@
 
 @WebMvcTest(EntityTypeController.class)
 class EntityTypeControllerTest {
-  private final static String GET_DEFINITION_URL = "/entity-types/{entity-type-id}";
   @Autowired
   private MockMvc mockMvc;
   @MockBean
   private EntityTypeService entityTypeService;
   @MockBean
   private FolioExecutionContext folioExecutionContext;
+  private final static String GET_DEFINITION_URL = "/entity-types/{entity-type-id}";
 
   @Test
   void shouldReturnEntityTypeDefinition() throws Exception {

diff --git a/src/test/java/org/folio/fqm/repository/DataRefreshRepositoryTest.java b/src/test/java/org/folio/fqm/repository/DataRefreshRepositoryTest.java
@@ -8,6 +8,7 @@
 import org.jooq.InsertValuesStep2;
 import org.jooq.Record;
 import org.jooq.Record2;
+import org.jooq.exception.DataAccessException;
 import org.jooq.impl.DSL;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -17,12 +18,15 @@
 import org.mockito.stubbing.Answer;
 
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 
 import static org.folio.fqm.repository.DataRefreshRepository.CURRENCY_FIELD;
 import static org.folio.fqm.repository.DataRefreshRepository.EXCHANGE_RATE_FIELD;
 import static org.jooq.impl.DSL.table;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.eq;
@@ -40,15 +44,45 @@ class DataRefreshRepositoryTest {
   @Mock
   private SimpleHttpClient simpleHttpClient;
 
+  @Test
+  void refreshMaterializedViewsConcurrentlyTest() {
+    String tenantId = "tenant_01";
+    List<String> viewsToRefresh = List.of("matview1", "matview2");
+    String expectedMatViewSql1 = "REFRESH MATERIALIZED VIEW CONCURRENTLY tenant_01_mod_fqm_manager.matview1";
+    String expectedMatViewSql2 = "REFRESH MATERIALIZED VIEW CONCURRENTLY tenant_01_mod_fqm_manager.matview2";
+    when(jooqContext.execute(anyString())).thenReturn(1);
+    List<String> failedRefreshes = dataRefreshRepository.refreshMaterializedViews(tenantId, viewsToRefresh, true);
+    verify(jooqContext, times(1)).execute(expectedMatViewSql1);
+    verify(jooqContext, times(1)).execute(expectedMatViewSql2);
+    assertTrue(failedRefreshes.isEmpty());
+  }
+
   @Test
   void refreshMaterializedViewsTest() {
     String tenantId = "tenant_01";
-    String expectedItemStatusSql = "REFRESH MATERIALIZED VIEW CONCURRENTLY tenant_01_mod_fqm_manager.drv_inventory_item_status";
-    String expectedLoanStatusSql = "REFRESH MATERIALIZED VIEW CONCURRENTLY tenant_01_mod_fqm_manager.drv_circulation_loan_status";
+    List<String> viewsToRefresh = List.of("matview1", "matview2");
+    String expectedMatViewSql1 = "REFRESH MATERIALIZED VIEW tenant_01_mod_fqm_manager.matview1";
+    String expectedMatViewSql2 = "REFRESH MATERIALIZED VIEW tenant_01_mod_fqm_manager.matview2";
     when(jooqContext.execute(anyString())).thenReturn(1);
-    dataRefreshRepository.refreshMaterializedViews(tenantId);
-    verify(jooqContext, times(1)).execute(expectedItemStatusSql);
-    verify(jooqContext, times(1)).execute(expectedLoanStatusSql);
+    List<String> failedRefreshes = dataRefreshRepository.refreshMaterializedViews(tenantId, viewsToRefresh, false);
+    verify(jooqContext, times(1)).execute(expectedMatViewSql1);
+    verify(jooqContext, times(1)).execute(expectedMatViewSql2);
+    assertTrue(failedRefreshes.isEmpty());
+  }
+
+  @Test
+  void shouldCatchExceptionWhenRefreshingMaterializedViews() {
+    String tenantId = "tenant_01";
+    List<String> viewsToRefresh = List.of("matview1", "matview2");
+    String expectedMatViewSql1 = "REFRESH MATERIALIZED VIEW tenant_01_mod_fqm_manager.matview1";
+    String expectedMatViewSql2 = "REFRESH MATERIALIZED VIEW tenant_01_mod_fqm_manager.matview2";
+    when(jooqContext.execute(expectedMatViewSql1)).thenReturn(1);
+    when(jooqContext.execute(expectedMatViewSql2)).thenThrow(DataAccessException.class);
+    List<String> failedRefreshes = dataRefreshRepository.refreshMaterializedViews(tenantId, viewsToRefresh, false);
+    verify(jooqContext, times(1)).execute(expectedMatViewSql1);
+    verify(jooqContext, times(1)).execute(expectedMatViewSql2);
+    assertFalse(failedRefreshes.contains("matview1"));
+    assertTrue(failedRefreshes.contains("matview2"));
   }
 
   @Test

diff --git a/src/test/java/org/folio/fqm/repository/ResultSetRepositoryTestDataProvider.java b/src/test/java/org/folio/fqm/repository/ResultSetRepositoryTestDataProvider.java
@@ -37,12 +37,6 @@ public class ResultSetRepositoryTestDataProvider implements MockDataProvider {
   public static final List<Map<String, Object>> TEST_ENTITY_WITH_ARRAY_CONTENTS = List.of(
     Map.of(ID_FIELD_NAME, UUID.randomUUID(), "testField", getPgArray()));
 
-  private static final EntityType ARRAY_ENTITY_TYPE = new EntityType()
-    .columns(List.of(
-      new EntityTypeColumn().name(ID_FIELD_NAME),
-      new EntityTypeColumn().name("testField").dataType(new EntityDataType().dataType("arrayType"))
-    ));
-
   private static final EntityType ENTITY_TYPE = new EntityType()
     .columns(List.of(
       new EntityTypeColumn().name(ID_FIELD_NAME).dataType(new RangedUUIDType().dataType("rangedUUIDType")).valueGetter(ID_FIELD_NAME).isIdColumn(true),