Fix(API): Fix usability of legacy API for application update. (#3029)

Legacy Application update API, when used with POST and PUT via Servlet,
invoke AbstractApplicationDefinition._createItem() method.
That's the only possible usage and it should work with legacy apps.

Updating an advanced application should always be rejected.
This API might be called within or without a transaction and we need a
consistent behavior.

Closes [BPM-124](https://bonitasoft.atlassian.net/browse/BPM-124)

bonita-integration-tests/bonita-integration-tests-web/src/test/java/org/bonitasoft/web/rest/server/api/application/APIApplicationIT.java

@@ -0,0 +1,198 @@
+/**
+ * Copyright (C) 2024 Bonitasoft S.A.
+ * Bonitasoft, 32 rue Gustave Eiffel - 38000 Grenoble
+ * This library is free software; you can redistribute it and/or modify it under the terms
+ * of the GNU Lesser General Public License as published by the Free Software Foundation
+ * version 2.1 of the License.
+ * This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU Lesser General Public License for more details.
+ * You should have received a copy of the GNU Lesser General Public License along with this
+ * program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth
+ * Floor, Boston, MA 02110-1301, USA.
+ **/
+package org.bonitasoft.web.rest.server.api.application;
+
+import static org.bonitasoft.web.rest.server.api.page.builder.PageItemBuilder.aPageItem;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.spy;
+
+import java.io.File;
+import java.net.URL;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.bonitasoft.console.common.server.preferences.constants.WebBonitaConstantsUtils;
+import org.bonitasoft.engine.api.ApplicationAPI;
+import org.bonitasoft.engine.api.PageAPI;
+import org.bonitasoft.engine.api.TenantAPIAccessor;
+import org.bonitasoft.engine.business.application.Application;
+import org.bonitasoft.engine.business.application.ApplicationImportPolicy;
+import org.bonitasoft.engine.page.Page;
+import org.bonitasoft.engine.search.SearchOptions;
+import org.bonitasoft.engine.search.SearchOptionsBuilder;
+import org.bonitasoft.engine.session.APISession;
+import org.bonitasoft.web.rest.model.application.AbstractApplicationItem;
+import org.bonitasoft.web.rest.model.application.AdvancedApplicationDefinition;
+import org.bonitasoft.web.rest.model.application.AdvancedApplicationItem;
+import org.bonitasoft.web.rest.model.application.ApplicationDefinition;
+import org.bonitasoft.web.rest.model.application.ApplicationItem;
+import org.bonitasoft.web.rest.model.portal.page.PageItem;
+import org.bonitasoft.web.rest.server.api.applicationpage.APIApplicationDataStoreFactory;
+import org.bonitasoft.web.rest.server.datastore.application.ApplicationDataStoreCreator;
+import org.bonitasoft.web.test.AbstractConsoleTest;
+import org.bonitasoft.web.toolkit.client.common.exception.api.APIException;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class APIApplicationIT extends AbstractConsoleTest {
+
+    public static final String HOME_PAGE_ZIP = "/homePage.zip";
+    public static final String LAYOUT_ZIP = "/layout.zip";
+    public static final String THEME_ZIP = "/theme.zip";
+    public static final String ADVANCED_APP_DESCRIPTOR = "/advancedAppDescriptor.xml";
+    private APIApplication apiApplication;
+    private APISession session;
+
+    @Override
+    public void consoleTestSetUp() throws Exception {
+        session = getInitiator().getSession();
+        apiApplication = spy(
+                new APIApplication(new ApplicationDataStoreCreator(), new APIApplicationDataStoreFactory()));
+        apiApplication.setCaller(getAPICaller(session, "API/living/application"));
+    }
+
+    private PageAPI getPageAPI() throws Exception {
+        return TenantAPIAccessor.getCustomPageAPI(session);
+    }
+
+    private ApplicationAPI getApplicationAPI() throws Exception {
+        return TenantAPIAccessor.getApplicationAPI(session);
+    }
+
+    @After
+    public void cleanPagesAndApplications() throws Exception {
+        final SearchOptions searchOptions = new SearchOptionsBuilder(0, 100000).done();
+
+        List<Application> apps = getApplicationAPI().searchApplications(searchOptions).getResult();
+        apps.stream().map(Application::getId).forEach(t -> {
+            try {
+                getApplicationAPI().deleteApplication(t);
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+        });
+
+        List<Page> pages = getPageAPI().searchPages(searchOptions).getResult();
+        var ids = pages.stream().map(Page::getId).toList();
+        getPageAPI().deletePages(ids);
+    }
+
+    private PageItem addPage(String pageFileName) throws Exception {
+        final PageItem pageItem = aPageItem().build();
+        final URL zipFileUrl = getClass().getResource(pageFileName);
+
+        final File zipFile = new File(zipFileUrl.toURI());
+        FileUtils.copyFileToDirectory(zipFile, WebBonitaConstantsUtils.getTenantInstance().getTempFolder());
+
+        final byte[] pageContent = FileUtils.readFileToByteArray(new File(zipFileUrl.toURI()));
+        return addPageItemToRepository(pageItem.getContentName(), pageContent);
+    }
+
+    private PageItem addPageItemToRepository(final String pageContentName, final byte[] pageContent) throws Exception {
+        return aPageItem().fromEngineItem(getPageAPI().createPage(pageContentName, pageContent)).build();
+    }
+
+    @Test
+    public void add_AdvancedApplication_is_forbidden() {
+        // Given
+        final AdvancedApplicationItem advancedItem = AdvancedApplicationDefinition.get().createItem();
+        advancedItem.setToken("tokenAdvanced");
+        advancedItem.setDisplayName("Advanced");
+        advancedItem.setVersion("1.0");
+        advancedItem.setProfileId(2L);
+        advancedItem.setState("Activated");
+
+        // When, Then exception
+        assertThrows("Expected exception: This deprecated API is not supported for advanced applications.",
+                APIException.class, () -> apiApplication.add(advancedItem));
+    }
+
+    @Test
+    public void should_add_LegacyApplication() throws Exception {
+        // Given
+        final PageItem pageItem = addPage(HOME_PAGE_ZIP);
+        final PageItem layout = addPage(LAYOUT_ZIP);
+        final PageItem theme = addPage(THEME_ZIP);
+        final ApplicationItem legacyItem = ApplicationDefinition.get().createItem();
+        legacyItem.setToken("tokenLegacy");
+        legacyItem.setDisplayName("Legacy");
+        legacyItem.setVersion("1.0");
+        legacyItem.setProfileId(2L);
+        legacyItem.setState("ACTIVATED");
+        // That's the default and gets saved as -1
+        // legacyItem.setHomePageId(pageItem.getId().toLong());
+        legacyItem.setLayoutId(layout.getId().toLong());
+        legacyItem.setThemeId(theme.getId().toLong());
+
+        // When
+        var createdItem = apiApplication.add(legacyItem);
+
+        // Then
+        Map<String, String> attributes = new HashMap<>(legacyItem.getAttributes().size());
+        legacyItem.getAttributes().keySet().forEach(k -> attributes.put(k, createdItem.getAttributes().get(k)));
+        Assert.assertEquals(new HashMap<>(legacyItem.getAttributes()), attributes);
+    }
+
+    @Test
+    public void update_AdvancedApplication_is_forbidden_and_not_effective() throws Exception {
+        // Given
+        getApplicationAPI().importApplications(
+                IOUtils.toByteArray(getClass().getResourceAsStream(ADVANCED_APP_DESCRIPTOR)),
+                ApplicationImportPolicy.REPLACE_DUPLICATES);
+        AdvancedApplicationItem advancedItem = (AdvancedApplicationItem) apiApplication
+                .search(0, 1, null, null, Collections.singletonMap("token", "app1")).getResults().get(0);
+        Map<String, String> attributes = Map.of(AbstractApplicationItem.ATTRIBUTE_DISPLAY_NAME, "Advanced Updated");
+
+        // When, Then exception
+        assertThrows(
+                "Expected exception: This deprecated API is not supported for advanced applications. The update has been applied nonetheless.",
+                APIException.class, () -> apiApplication.update(advancedItem.getId(), attributes));
+        // Then not updated
+        assertEquals("Application 1", apiApplication.get(advancedItem.getId()).getDisplayName());
+    }
+
+    @Test
+    public void should_update_LegacyApplication() throws Exception {
+        // Given
+        final PageItem pageItem = addPage(HOME_PAGE_ZIP);
+        final PageItem layout = addPage(LAYOUT_ZIP);
+        final PageItem theme = addPage(THEME_ZIP);
+        final ApplicationItem legacyItem = ApplicationDefinition.get().createItem();
+        legacyItem.setToken("tokenLegacy");
+        legacyItem.setDisplayName("Legacy");
+        legacyItem.setVersion("1.0");
+        legacyItem.setProfileId(2L);
+        legacyItem.setState("ACTIVATED");
+        // That's the default and gets saved as -1
+        // legacyItem.setHomePageId(pageItem.getId().toLong());
+        legacyItem.setLayoutId(layout.getId().toLong());
+        legacyItem.setThemeId(theme.getId().toLong());
+
+        var createdItem = apiApplication.add(legacyItem);
+        Map<String, String> attributes = Map.of(AbstractApplicationItem.ATTRIBUTE_DISPLAY_NAME, "Legacy Updated");
+
+        // When
+        var updatedItem = apiApplication.update(createdItem.getId(), attributes);
+
+        // Then
+        assertEquals("Legacy Updated", updatedItem.getDisplayName());
+    }
+
+}

bonita-integration-tests/bonita-integration-tests-web/src/test/resources/advancedAppDescriptor.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<applications xmlns="http://documentation.bonitasoft.com/application-xml-schema/1.1">
+  <advancedApplication token="app1" version="1.0" profile="User" state="ACTIVATED">
+    <displayName>Application 1</displayName>
+    <description>Description of Application 1</description>
+    <iconPath>/app1.jpg</iconPath>
+  </advancedApplication>
+</applications>

bpm/bonita-core/bonita-process-engine/src/main/java/org/bonitasoft/engine/api/impl/livingapplication/LivingApplicationAPIDelegate.java

@@ -13,6 +13,8 @@
  **/
 package org.bonitasoft.engine.api.impl.livingapplication;
 
+import java.util.Optional;
+
 import org.bonitasoft.engine.api.impl.converter.ApplicationModelConverter;
 import org.bonitasoft.engine.business.application.*;
 import org.bonitasoft.engine.business.application.impl.IconImpl;
@@ -151,6 +153,15 @@ public Application updateApplication(final long applicationId, final Application
             validateUpdater(updater);
             AbstractSApplication application;
             if (!updater.getFields().isEmpty()) {
+                /*
+                 * This API may be called within our without a transaction.
+                 * So we must check first whether the application is advanced to have a consistent behavior
+                 * and never update the advanced application.
+                 */
+                if (Optional.ofNullable(applicationService.getApplicationWithIcon(applicationId))
+                        .filter(AbstractSApplication::isAdvanced).isPresent()) {
+                    throw new UpdateException("This deprecated API is not supported for advanced applications.");
+                }
                 application = applicationService.updateApplication(applicationId,
                         converter.toApplicationUpdateDescriptor(updater, loggedUserId));
             } else {
@@ -160,8 +171,7 @@ public Application updateApplication(final long applicationId, final Application
             if (converted instanceof Application res) {
                 return res;
             } else {
-                throw new UpdateException(
-                        "This deprecated API is not supported for advanced applications. The update has been applied nonetheless.");
+                throw new UpdateException("This deprecated API is not supported for advanced applications.");
             }
         } catch (final SObjectAlreadyExistsException e) {
             throw new AlreadyExistsException(e.getMessage());

bpm/bonita-web-server/src/main/java/org/bonitasoft/web/rest/model/application/AbstractApplicationDefinition.java

@@ -66,8 +66,8 @@ public static AbstractApplicationDefinition<? extends AbstractApplicationItem> g
 
     @Override
     protected ITEM _createItem() {
-        throw new UnsupportedOperationException(
-                "The _createItem method should not be called for this abstract class. Use subclasses to instantiate.");
+        // this might be called by deprecated PUT and POST methods which work only with legacy applications...
+        return (ITEM) ApplicationDefinition.get()._createItem();
     }
 
 }

bpm/bonita-web-server/src/test/java/org/bonitasoft/web/rest/server/api/application/APIApplicationTest.java

@@ -41,6 +41,7 @@
 import org.bonitasoft.web.rest.server.framework.Deployer;
 import org.bonitasoft.web.rest.server.framework.search.ItemSearchResult;
 import org.bonitasoft.web.toolkit.client.ItemDefinitionFactory;
+import org.bonitasoft.web.toolkit.client.data.APIID;
 import org.bonitasoft.web.toolkit.client.data.item.ItemDefinition;
 import org.junit.Before;
 import org.junit.Test;
@@ -164,4 +165,19 @@ public void fillDeploys_should_add_deployers_for_createdBy_updatedBy_ProfileId_a
         assertThat(deployers.get(ApplicationItem.ATTRIBUTE_THEME_ID)).isExactlyInstanceOf(PageDeployer.class);
     }
 
+    @Test
+    public void update_should_return_the_appropriate_application_kind() throws Exception {
+        //given
+        final APIID idToUpdate = mock(APIID.class);
+        final ApplicationItem updatedItem = mock(ApplicationItem.class);
+        Map<String, String> attributes = Collections.emptyMap();
+        given(dataStore.update(idToUpdate, attributes)).willReturn(updatedItem);
+
+        //when
+        final ApplicationItem retrievedItem = apiApplication.update(idToUpdate, attributes);
+
+        //then
+        assertThat(retrievedItem).isEqualTo(updatedItem);
+    }
+
 }