Fixed testability of PaygUpdateAuthTask

java/code/src/com/redhat/rhn/manager/content/test/ContentSyncManagerPaygTest.java

@@ -32,12 +32,14 @@
 import com.redhat.rhn.domain.scc.SCCCachingFactory;
 import com.redhat.rhn.domain.scc.SCCRepository;
 import com.redhat.rhn.domain.scc.SCCRepositoryAuth;
+import com.redhat.rhn.manager.content.ContentSyncException;
 import com.redhat.rhn.manager.content.ContentSyncManager;
 import com.redhat.rhn.taskomatic.task.payg.PaygAuthDataExtractor;
 import com.redhat.rhn.taskomatic.task.payg.PaygUpdateAuthTask;
 import com.redhat.rhn.taskomatic.task.payg.beans.PaygInstanceInfo;
 import com.redhat.rhn.taskomatic.task.payg.beans.PaygProductInfo;
 import com.redhat.rhn.taskomatic.task.payg.test.PaygUpdateAuthTaskTest;
+import com.redhat.rhn.testing.MockFileLocks;
 import com.redhat.rhn.testing.RhnBaseTestCase;
 import com.redhat.rhn.testing.TestUtils;
 
@@ -101,6 +103,15 @@ protected PaygInstanceInfo extractAuthDataLocal() {
             }
         };
         PAYG_DATA_TASK.setPaygDataExtractor(paygAuthDataExtractorMock);
+
+        ContentSyncManager contentSyncManager = new ContentSyncManager() {
+            @Override
+            public void updateRepositories(String mirrorUrl) throws ContentSyncException {
+                // Nothing to do
+            }
+        };
+        PAYG_DATA_TASK.setContentSyncManager(contentSyncManager);
+        PAYG_DATA_TASK.setSccRefreshLock(new MockFileLocks());
     }
 
     @Override

java/code/src/com/redhat/rhn/taskomatic/task/payg/PaygUpdateAuthTask.java

@@ -48,6 +48,10 @@ public class PaygUpdateAuthTask extends RhnJavaJob {
 
     private CloudPaygManager cloudPaygManager = GlobalInstanceHolder.PAYG_MANAGER;
 
+    private ContentSyncManager contentSyncManager = new ContentSyncManager();
+
+    private FileLocks sccRefreshLock = FileLocks.SCC_REFRESH_LOCK;
+
     private static final String KEY_ID = "sshData_id";
 
     @Override
@@ -84,7 +88,7 @@ public void execute(JobExecutionContext jobExecutionContext) {
 
         manageLocalHostPayg();
 
-        FileLocks.SCC_REFRESH_LOCK.withFileLock(() -> {
+        sccRefreshLock.withFileLock(() -> {
             if (jobExecutionContext != null && jobExecutionContext.getJobDetail().getJobDataMap().containsKey(KEY_ID)) {
                 Optional<PaygSshData> paygData = PaygSshDataFactory.lookupById(
                         Integer.parseInt((String) jobExecutionContext.getJobDetail().getJobDataMap().get(KEY_ID)));
@@ -97,7 +101,6 @@ public void execute(JobExecutionContext jobExecutionContext) {
 
             // Call the content sync manager to refresh all repositories content sources and the authorizations
             try {
-                ContentSyncManager contentSyncManager = new ContentSyncManager();
                 contentSyncManager.updateRepositories(null);
             }
             catch (ContentSyncException ex) {
@@ -115,13 +118,29 @@ public void setPaygDataExtractor(PaygAuthDataExtractor paygDataExtractorIn) {
     }
 
     /**
-     * Need for automatic tests
+     * Needed for unit tests
      * @param mgrIn
      */
     public void setCloudPaygManager(CloudPaygManager mgrIn) {
         cloudPaygManager = mgrIn;
     }
 
+    /**
+     * Needed for unit tests
+     * @param contentSyncManagerIn
+     */
+    public void setContentSyncManager(ContentSyncManager contentSyncManagerIn) {
+        this.contentSyncManager = contentSyncManagerIn;
+    }
+
+    /**
+     * Needed for unit testing
+     * @param sccRefreshLockIn
+     */
+    public void setSccRefreshLock(FileLocks sccRefreshLockIn) {
+        this.sccRefreshLock = sccRefreshLockIn;
+    }
+
     private void updateInstanceData(PaygSshData instance) {
         try {
             PaygInstanceInfo paygData = paygDataExtractor.extractAuthData(instance);

java/code/src/com/redhat/rhn/taskomatic/task/payg/test/PaygUpdateAuthTaskTest.java

@@ -15,6 +15,7 @@
 
 package com.redhat.rhn.taskomatic.task.payg.test;
 
+import static com.redhat.rhn.testing.RhnBaseTestCase.assertContains;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -34,13 +35,15 @@
 import com.redhat.rhn.domain.org.OrgFactory;
 import com.redhat.rhn.domain.product.SUSEProductFactory;
 import com.redhat.rhn.domain.scc.SCCCachingFactory;
-import com.redhat.rhn.frontend.xmlrpc.test.BaseHandlerTestCase;
 import com.redhat.rhn.manager.content.ContentSyncManager;
 import com.redhat.rhn.taskomatic.task.payg.PaygAuthDataExtractor;
 import com.redhat.rhn.taskomatic.task.payg.PaygDataExtractException;
 import com.redhat.rhn.taskomatic.task.payg.PaygUpdateAuthTask;
 import com.redhat.rhn.taskomatic.task.payg.beans.PaygInstanceInfo;
 import com.redhat.rhn.taskomatic.task.payg.beans.PaygProductInfo;
+import com.redhat.rhn.testing.JMockBaseTestCaseWithUser;
+import com.redhat.rhn.testing.MockFileLocks;
+import com.redhat.rhn.testing.TestUtils;
 
 import com.suse.cloud.CloudPaygManager;
 
@@ -54,13 +57,9 @@
 import org.apache.commons.io.FileUtils;
 import org.jmock.Expectations;
 import org.jmock.imposters.ByteBuddyClassImposteriser;
-import org.jmock.junit5.JUnit5Mockery;
-import org.jmock.lib.concurrent.Synchroniser;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.junit.jupiter.api.extension.RegisterExtension;
 
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
@@ -76,11 +75,7 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
-@ExtendWith(JUnit5Mockery.class)
-public class PaygUpdateAuthTaskTest extends BaseHandlerTestCase {
-
-    private static PaygAuthDataExtractor paygAuthDataExtractorMock;
-    private static final PaygUpdateAuthTask PAYG_DATA_TASK = new PaygUpdateAuthTask();
+public class PaygUpdateAuthTaskTest extends JMockBaseTestCaseWithUser {
 
     private static final Gson GSON = new GsonBuilder()
             .serializeNulls()
@@ -90,27 +85,34 @@ public class PaygUpdateAuthTaskTest extends BaseHandlerTestCase {
             "/com/redhat/rhn/manager/content/test/smallBase/productsUnscoped.json";
     private static final String PRODUCT_TREE = "/com/redhat/rhn/manager/content/test/smallBase/product_tree.json";
 
-    @RegisterExtension
-    protected static final JUnit5Mockery CONTEXT = new JUnit5Mockery() {{
-        setThreadingPolicy(new Synchroniser());
-    }};
+    private PaygAuthDataExtractor paygAuthDataExtractorMock;
 
-    static {
-        CONTEXT.setImposteriser(ByteBuddyClassImposteriser.INSTANCE);
-        paygAuthDataExtractorMock = CONTEXT.mock(PaygAuthDataExtractor.class);
-        PAYG_DATA_TASK.setPaygDataExtractor(paygAuthDataExtractorMock);
-    }
+    private PaygUpdateAuthTask paygUpdateAuthTask;
 
     private PaygSshData paygData;
     private PaygInstanceInfo paygInstanceInfo;
+    private ContentSyncManager contentSyncManagerMock;
 
     @Override
     @BeforeEach
     public void setUp() throws Exception {
         super.setUp();
         clearDb();
 
-        PAYG_DATA_TASK.setCloudPaygManager(new CloudPaygManager());
+        setImposteriser(ByteBuddyClassImposteriser.INSTANCE);
+
+        paygAuthDataExtractorMock = mock(PaygAuthDataExtractor.class);
+        contentSyncManagerMock = mock(ContentSyncManager.class);
+
+        checking(new Expectations() {{ allowing(contentSyncManagerMock).updateRepositories(null); }});
+
+        paygUpdateAuthTask = new PaygUpdateAuthTask();
+
+        paygUpdateAuthTask.setCloudPaygManager(new CloudPaygManager());
+        paygUpdateAuthTask.setPaygDataExtractor(paygAuthDataExtractorMock);
+        paygUpdateAuthTask.setContentSyncManager(contentSyncManagerMock);
+        paygUpdateAuthTask.setSccRefreshLock(new MockFileLocks());
+
         paygData = createPaygSshData();
         PaygSshDataFactory.savePaygSshData(paygData);
         paygInstanceInfo = createPaygInstanceInfo();
@@ -138,7 +140,7 @@ private void clearDb() {
 
     @Test
     public void testLocalhostPaygConnection() throws Exception {
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(returnValue(paygInstanceInfo));
@@ -153,8 +155,8 @@ public boolean isPaygInstance() {
                 return true;
             }
         };
-        PAYG_DATA_TASK.setCloudPaygManager(mgr);
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.setCloudPaygManager(mgr);
+        paygUpdateAuthTask.execute(null);
         for (PaygSshData outPaygData : PaygSshDataFactory.lookupPaygSshData()) {
             switch (outPaygData.getHost()) {
                 case "localhost":
@@ -223,8 +225,8 @@ public boolean isPaygInstance() {
                 return false;
             }
         };
-        PAYG_DATA_TASK.setCloudPaygManager(mgr);
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.setCloudPaygManager(mgr);
+        paygUpdateAuthTask.execute(null);
         for (PaygSshData outPaygData : PaygSshDataFactory.lookupPaygSshData()) {
             switch (outPaygData.getHost()) {
                 case "my-instance":
@@ -239,12 +241,12 @@ public boolean isPaygInstance() {
     }
     public void testRHUIConnection() throws Exception {
         PaygInstanceInfo rhuiInstanceInfo = createRHUIInstanceInfo();
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(returnValue(rhuiInstanceInfo));
             }});
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         for (PaygSshData outPaygData : PaygSshDataFactory.lookupPaygSshData()) {
             Credentials creds = outPaygData.getCredentials();
             assertEquals("rhui", creds.getType().getLabel());
@@ -305,12 +307,12 @@ else if (cs.getLabel().equals("repo-label-2-c" + cid)) {
 
     @Test
     public void testJschException() throws Exception {
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(throwException(new JSchException("My JSchException exception")));
             }});
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
         assertContains(paygData.getErrorMessage(), "My JSchException exception");
         assertEquals(paygData.getStatus(), PaygSshData.Status.E);
@@ -321,12 +323,12 @@ public void testJschException() throws Exception {
 
     @Test
     public void testPaygDataExtractException() throws Exception {
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(throwException(new PaygDataExtractException("My PaygDataExtractException")));
             }});
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
         assertContains(paygData.getErrorMessage(), "My PaygDataExtractException");
         assertEquals(paygData.getStatus(), PaygSshData.Status.E);
@@ -343,7 +345,7 @@ public void testPaygDataExtractException() throws Exception {
      */
     @Test
     public void testPaygDataExtractExceptionInvalidateCredentials() throws Exception {
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(returnValue(paygInstanceInfo));
@@ -355,7 +357,7 @@ public void testPaygDataExtractExceptionInvalidateCredentials() throws Exception
                 will(returnValue(paygInstanceInfo));
             }});
         // first call successfull - set credentials and a header
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
         assertEquals(paygData.getStatus(), PaygSshData.Status.S);
         Credentials creds = paygData.getCredentials();
@@ -365,7 +367,7 @@ public void testPaygDataExtractExceptionInvalidateCredentials() throws Exception
                 new String(creds.getExtraAuthData()));
 
         // second call failed - set status to Error, but keep credentials
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
 
         assertContains(paygData.getErrorMessage(), "My PaygDataExtractException");
@@ -380,7 +382,7 @@ public void testPaygDataExtractExceptionInvalidateCredentials() throws Exception
                 new String(creds.getExtraAuthData()));
 
         // third call failed - invalidate credentials
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
 
         assertContains(paygData.getErrorMessage(), "My PaygDataExtractException");
@@ -391,7 +393,7 @@ public void testPaygDataExtractExceptionInvalidateCredentials() throws Exception
         assertEquals("{}", new String(creds.getExtraAuthData()));
 
         // forth call successfull - restore credentials and a header again
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
         assertEquals(paygData.getStatus(), PaygSshData.Status.S);
         creds = paygData.getCredentials();
@@ -403,13 +405,13 @@ public void testPaygDataExtractExceptionInvalidateCredentials() throws Exception
 
     @Test
     public void testGenericException() throws Exception {
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
 
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(throwException(new Exception("My Exception")));
             }});
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
         assertNull(paygData.getErrorMessage());
         assertEquals(paygData.getStatus(), PaygSshData.Status.E);
@@ -420,12 +422,12 @@ public void testGenericException() throws Exception {
 
     @Test
     public void testSuccessClearStatus() throws Exception {
-        CONTEXT.checking(new Expectations() {
+        checking(new Expectations() {
             {
                 oneOf(paygAuthDataExtractorMock).extractAuthData(with(any(PaygSshData.class)));
                 will(returnValue(paygInstanceInfo));
             }});
-        PAYG_DATA_TASK.execute(null);
+        paygUpdateAuthTask.execute(null);
         paygData = HibernateFactory.reload(paygData);
         assertNull(paygData.getErrorMessage());
         assertEquals(paygData.getStatus(), PaygSshData.Status.S);

java/code/src/com/redhat/rhn/testing/MockFileLocks.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2023 SUSE LLC
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+
+package com.redhat.rhn.testing;
+
+import com.redhat.rhn.common.util.FileLocks;
+
+import java.util.function.Supplier;
+
+/**
+ * Mock object for replacing {@link FileLocks} in unit tests.
+ */
+public class MockFileLocks extends FileLocks {
+    /**
+     * Default constructor
+     */
+    public MockFileLocks() {
+        super("dummy.lock");
+    }
+
+    @Override
+    public <T> T withFileLock(Supplier<T> fn) {
+        return fn.get();
+    }
+}