Reload permissions immediately on enable (not upgrade) OKAPI-996 (#1101)

Reload permissions immediately on enable (not upgrade) OKAPI-996 1. reload permissions when mod-permissions is being enabled (not upgraded) 2. reload permissions at end of install when mod-permissions is upgraded. Previously permissions was reloaded at the end always.
folio-org · Mar 8, 2021 · 4efffa5 · 4efffa5
1 parent 519ef09
commit 4efffa5
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 26 deletions.
diff --git a/okapi-core/src/main/java/org/folio/okapi/managers/TenantManager.java b/okapi-core/src/main/java/org/folio/okapi/managers/TenantManager.java
@@ -275,25 +275,19 @@ private Future<String> enableAndDisableModule(
               ? enableAndDisableCheck(tenant, mdFrom.result(), mdTo.result())
               : Future.succeededFuture())
         .compose(x -> enableAndDisableModule(tenant, options, mdFrom.result(),
-            mdTo.result(), true, pc));
+            mdTo.result(), pc));
   }
 
   private Future<String> enableAndDisableModule(Tenant tenant, TenantInstallOptions options,
                                                 ModuleDescriptor mdFrom, ModuleDescriptor mdTo,
-                                                boolean reload, ProxyContext pc) {
+                                                ProxyContext pc) {
     if (mdFrom == null && mdTo == null) {
       return Future.succeededFuture("");
     }
     return invokePermissions(tenant, options, mdFrom, mdTo, pc)
         .compose(x -> invokeTenantInterface(tenant, options, mdFrom, mdTo, pc))
         .compose(x -> invokePermissionsPermMod(tenant, options, mdTo, pc))
-        .compose(x -> {
-          if (reload) {
-            return reloadPermissions(tenant, options, mdTo, pc);
-          } else {
-            return Future.succeededFuture();
-          }
-        })
+        .compose(x -> reloadPermissions(tenant, options, mdFrom, mdTo, pc))
         .compose(x -> commitModuleChange(tenant, mdFrom, mdTo))
         .compose(x -> Future.succeededFuture((mdTo != null ? mdTo.getId() : ""))
     );
@@ -444,24 +438,29 @@ private Future<Void> invokePermissionsPermMod(Tenant tenant, TenantInstallOption
   }
 
   /**
-   * Announce permissions for a set of modules to a permissions module.
+   * Conditionally announce permissions for a set of modules to a permissions module.
+   *
+   * <p>This will only happen if the permissions module is being enabled for the first time,
+   * not when it's upgraded.
    *
    * @param tenant tenant
-   * @param mdTo permissions module
+   * @param mdFrom permissions module from
+   * @param mdTo permissions module to
    * @param pc ProxyContext
    * @return future
    */
   private Future<Void> reloadPermissions(Tenant tenant, TenantInstallOptions options,
-                                         ModuleDescriptor mdTo, ProxyContext pc) {
+                                         ModuleDescriptor mdFrom, ModuleDescriptor mdTo,
+                                         ProxyContext pc) {
 
-    if (mdTo == null || !options.checkInvoke(mdTo.getId())
-        || mdTo.getSystemInterface("_tenantPermissions") == null) {
-      return Future.succeededFuture();
-    }
     Future<Void> future = Future.succeededFuture();
-    for (String mdid : tenant.listModules()) {
-      future = future.compose(x -> moduleManager.get(mdid)
-          .compose(md -> invokePermissionsForModule(tenant, null, md, mdTo, pc)));
+    // only reload if mod-permissions is being enabled (not when upgraded)
+    if (mdFrom == null && mdTo != null && options.checkInvoke(mdTo.getId())
+        && mdTo.getSystemInterface("_tenantPermissions") != null) {
+      for (String mdid : tenant.listModules()) {
+        future = future.compose(x -> moduleManager.get(mdid)
+            .compose(md -> invokePermissionsForModule(tenant, null, md, mdTo, pc)));
+      }
     }
     return future;
   }
@@ -1074,10 +1073,12 @@ private Future<List<TenantModuleDescriptor>> runJob(
             return jobs.put(t.getId(), job.getId(), job);
           });
         }
+        // if we are really upgrading permissions do a refresh last
         for (TenantModuleDescriptor tm : tml) {
-          if (tm.getAction() == Action.enable) {
+          if (tm.getAction() == Action.enable && tm.getFrom() != null) {
             ModuleDescriptor mdTo = modsAvailable.get(tm.getId());
-            future = future.compose(x -> reloadPermissions(t, options, mdTo, pc));
+            // mdFrom is null so reloadPermissions is triggered!!!!
+            future = future.compose(x -> reloadPermissions(t, options, null, mdTo, pc));
           }
         }
         if (options.getDeploy()) {
@@ -1145,7 +1146,7 @@ private Future<Void> installTenantModule(Tenant tenant, ProxyContext pc,
     } else if (tm.getAction() == Action.disable) {
       mdFrom = modsAvailable.get(tm.getId());
     }
-    return enableAndDisableModule(tenant, options, mdFrom, mdTo, false, pc)
+    return enableAndDisableModule(tenant, options, mdFrom, mdTo, pc)
         .onFailure(x -> tm.setMessage(x.getMessage()))
         .mapEmpty();
   }

diff --git a/okapi-core/src/test/java/org/folio/okapi/ModuleTest.java b/okapi-core/src/test/java/org/folio/okapi/ModuleTest.java
@@ -2698,11 +2698,10 @@ public void testInternalModule(TestContext context) {
           .get("/permResult")
           .then()
           .statusCode(200)
-          .body("$", hasSize(4))
+          .body("$", hasSize(3))
           .body("[0].moduleId", is("header-1"))
-          .body("[1].moduleId", is("header-1"))
-          .body("[2].moduleId", is("okapi-0.0.0"))
-          .body("[3].moduleId", is("sample-module-1.0.0"));
+          .body("[1].moduleId", is("okapi-0.0.0"))
+          .body("[2].moduleId", is("sample-module-1.0.0"));
     }
 
     conf.put("okapiVersion", "3.0.0");  // upgrade from 0.0.0 to 3.0.0