fix(provider): rename getProviderByName to getProviderByType

Author: christophechevalier

src/main/java/io/github/linagora/linid/im/plugin/entity/DynamicEntityServiceImpl.java

@@ -299,7 +299,8 @@ public Page<DynamicEntity> handleFindAll(HttpServletRequest request, String enti
    * @throws ApiException if the provider plugin is unknown
    */
   public ProviderPlugin getProvider(DynamicEntity entity) {
-    return providerFactory.getProviderByName(entity.getConfiguration().getProvider())
+    var providerConfiguration = getProviderConfiguration(entity);
+    return providerFactory.getProviderByType(providerConfiguration.getType())
         .orElseThrow(() -> new ApiException(500, I18nMessage.of("error.provider.unknown",
             Map.of(
                 ENTITY_KEYWORD, entity.getConfiguration().getName(),

src/main/java/io/github/linagora/linid/im/plugin/provider/ProviderFactoryImpl.java

@@ -45,14 +45,14 @@
 public class ProviderFactoryImpl implements ProviderFactory {
   /**
    * Registry containing all available {@link ProviderPlugin} instances. Used to resolve the appropriate plugin by matching its
-   * supported name.
+   * supported type.
    */
   private final PluginRegistry<ProviderPlugin, String> providerRegistry;
 
   @Override
-  public Optional<ProviderPlugin> getProviderByName(final String name) {
+  public Optional<ProviderPlugin> getProviderByType(final String type) {
     return providerRegistry.getPlugins().stream()
-        .filter(provider -> provider.supports(name))
+        .filter(provider -> provider.supports(type))
         .findFirst();
   }
 }

src/main/java/io/github/linagora/linid/im/plugin/task/TaskEngineImpl.java

@@ -67,10 +67,10 @@ public class TaskEngineImpl implements TaskEngine {
   public void execute(DynamicEntity dynamicEntity, TaskExecutionContext context, String phase) {
     dynamicEntity.getConfiguration().getTasks()
         .stream()
-        .filter(configuration -> configuration.getPhases().contains(phase))
+        .filter(task -> task.getPhases().contains(phase))
         .map(this::mergeConfigurationWithGlobal)
-        .forEach(configuration -> getPlugin(configuration)
-            .execute(configuration, dynamicEntity, context));
+        .forEach(task -> getPlugin(task)
+            .execute(task, dynamicEntity, context));
   }
 
   /**
@@ -83,7 +83,7 @@ public void execute(DynamicEntity dynamicEntity, TaskExecutionContext context, S
   public TaskPlugin getPlugin(TaskConfiguration configuration) {
     return taskRegistry.getPlugins()
         .stream()
-        .filter(validationPlugin -> validationPlugin.supports(configuration.getName()))
+        .filter(taskPlugin -> taskPlugin.supports(configuration.getType()))
         .findFirst()
         .orElseThrow(() -> new ApiException(400, I18nMessage.of(
             "error.plugin.unknown",

src/test/java/io/github/linagora/linid/im/plugin/entity/DynamicEntityServiceImplTest.java

@@ -77,6 +77,7 @@ void testHandleCreate() {
     var entityConfiguration = new EntityConfiguration();
     entityConfiguration.setProvider("test");
     var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("test-type");
     var provider = Mockito.mock(ProviderPlugin.class);
     var request = Mockito.mock(HttpServletRequest.class);
     var authPlugin = Mockito.mock(AllowAllAuthorizationPlugin.class);
@@ -85,7 +86,7 @@ void testHandleCreate() {
     Mockito.when(configurationService.getEntityConfiguration(Mockito.anyString())).thenReturn(Optional.of(entityConfiguration));
     Mockito.when(configurationService.getProviderConfiguration(Mockito.any()))
         .thenReturn(Optional.of(providerConfiguration));
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.of(provider));
+    Mockito.when(providerFactory.getProviderByType(Mockito.anyString())).thenReturn(Optional.of(provider));
     Mockito.doNothing().when(taskEngine).execute(Mockito.any(), Mockito.any(), Mockito.any());
     Mockito.doNothing().when(validationEngine).validate(Mockito.any(), Mockito.any());
     Mockito.when(provider.create(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(null);
@@ -119,6 +120,7 @@ void testHandleUpdate() {
     var entityConfiguration = new EntityConfiguration();
     entityConfiguration.setProvider("test");
     var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("test-type");
     var provider = Mockito.mock(ProviderPlugin.class);
     var request = Mockito.mock(HttpServletRequest.class);
     var authPlugin = Mockito.mock(AllowAllAuthorizationPlugin.class);
@@ -127,7 +129,7 @@ void testHandleUpdate() {
     Mockito.when(configurationService.getEntityConfiguration(Mockito.anyString())).thenReturn(Optional.of(entityConfiguration));
     Mockito.when(configurationService.getProviderConfiguration(Mockito.any()))
         .thenReturn(Optional.of(providerConfiguration));
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.of(provider));
+    Mockito.when(providerFactory.getProviderByType(Mockito.anyString())).thenReturn(Optional.of(provider));
     Mockito.doNothing().when(taskEngine).execute(Mockito.any(), Mockito.any(), Mockito.any());
     Mockito.doNothing().when(validationEngine).validate(Mockito.any(), Mockito.any());
     Mockito.when(provider.update(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(null);
@@ -163,6 +165,7 @@ void testHandlePatch() {
     var entityConfiguration = new EntityConfiguration();
     entityConfiguration.setProvider("test");
     var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("test-type");
     var provider = Mockito.mock(ProviderPlugin.class);
     var request = Mockito.mock(HttpServletRequest.class);
     var authPlugin = Mockito.mock(AllowAllAuthorizationPlugin.class);
@@ -171,7 +174,7 @@ void testHandlePatch() {
     Mockito.when(configurationService.getEntityConfiguration(Mockito.anyString())).thenReturn(Optional.of(entityConfiguration));
     Mockito.when(configurationService.getProviderConfiguration(Mockito.any()))
         .thenReturn(Optional.of(providerConfiguration));
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.of(provider));
+    Mockito.when(providerFactory.getProviderByType(Mockito.anyString())).thenReturn(Optional.of(provider));
     Mockito.doNothing().when(taskEngine).execute(Mockito.any(), Mockito.any(), Mockito.any());
     Mockito.doNothing().when(validationEngine).validate(Mockito.any(), Mockito.any());
     Mockito.when(provider.patch(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(null);
@@ -207,6 +210,7 @@ void testHandleDelete() {
     var entityConfiguration = new EntityConfiguration();
     entityConfiguration.setProvider("test");
     var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("test-type");
     var provider = Mockito.mock(ProviderPlugin.class);
     var request = Mockito.mock(HttpServletRequest.class);
     var authPlugin = Mockito.mock(AllowAllAuthorizationPlugin.class);
@@ -215,7 +219,7 @@ void testHandleDelete() {
     Mockito.when(configurationService.getEntityConfiguration(Mockito.anyString())).thenReturn(Optional.of(entityConfiguration));
     Mockito.when(configurationService.getProviderConfiguration(Mockito.any()))
         .thenReturn(Optional.of(providerConfiguration));
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.of(provider));
+    Mockito.when(providerFactory.getProviderByType(Mockito.anyString())).thenReturn(Optional.of(provider));
     Mockito.doNothing().when(taskEngine).execute(Mockito.any(), Mockito.any(), Mockito.any());
     Mockito.doNothing().when(validationEngine).validate(Mockito.any(), Mockito.any());
     Mockito.when(provider.delete(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(true);
@@ -251,6 +255,7 @@ void testHandleFindById() {
     var entityConfiguration = new EntityConfiguration();
     entityConfiguration.setProvider("test");
     var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("test-type");
     var provider = Mockito.mock(ProviderPlugin.class);
     var request = Mockito.mock(HttpServletRequest.class);
     var authPlugin = Mockito.mock(AllowAllAuthorizationPlugin.class);
@@ -259,7 +264,7 @@ void testHandleFindById() {
     Mockito.when(configurationService.getEntityConfiguration(Mockito.anyString())).thenReturn(Optional.of(entityConfiguration));
     Mockito.when(configurationService.getProviderConfiguration(Mockito.any()))
         .thenReturn(Optional.of(providerConfiguration));
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.of(provider));
+    Mockito.when(providerFactory.getProviderByType(Mockito.anyString())).thenReturn(Optional.of(provider));
     Mockito.doNothing().when(taskEngine).execute(Mockito.any(), Mockito.any(), Mockito.any());
     Mockito.doNothing().when(validationEngine).validate(Mockito.any(), Mockito.any());
     Mockito.when(provider.findById(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(null);
@@ -295,6 +300,7 @@ void testHandleFindAll() {
     var entityConfiguration = new EntityConfiguration();
     entityConfiguration.setProvider("test");
     var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("test-type");
     var provider = Mockito.mock(ProviderPlugin.class);
     var request = Mockito.mock(HttpServletRequest.class);
     var authPlugin = Mockito.mock(AllowAllAuthorizationPlugin.class);
@@ -303,7 +309,7 @@ void testHandleFindAll() {
     Mockito.when(configurationService.getEntityConfiguration(Mockito.anyString())).thenReturn(Optional.of(entityConfiguration));
     Mockito.when(configurationService.getProviderConfiguration(Mockito.any()))
         .thenReturn(Optional.of(providerConfiguration));
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.of(provider));
+    Mockito.when(providerFactory.getProviderByType(Mockito.anyString())).thenReturn(Optional.of(provider));
     Mockito.doNothing().when(taskEngine).execute(Mockito.any(), Mockito.any(), Mockito.any());
     Mockito.doNothing().when(validationEngine).validate(Mockito.any(), Mockito.any());
     Mockito.when(provider.findAll(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(null);
@@ -353,20 +359,52 @@ void testUpdateEntityConfiguration() {
   @Test
   @DisplayName("test getProvider: should throw exception without provider")
   void testGetProvider() {
-    Mockito.when(providerFactory.getProviderByName(Mockito.anyString())).thenReturn(Optional.empty());
+    var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setType("http");
+
+    Mockito.when(configurationService.getProviderConfiguration("my-provider"))
+        .thenReturn(Optional.of(providerConfiguration));
+    Mockito.when(providerFactory.getProviderByType("http")).thenReturn(Optional.empty());
 
     var entityConfiguration = new EntityConfiguration();
-    entityConfiguration.setProvider("provider");
+    entityConfiguration.setProvider("my-provider");
     entityConfiguration.setName("name");
     var entity = new DynamicEntity();
     entity.setConfiguration(entityConfiguration);
 
     ApiException ex = assertThrows(ApiException.class, () -> service.getProvider(entity));
     assertEquals("error.provider.unknown", ex.getError().key());
-    assertEquals(Map.of("entity", "name", "provider", "provider"), ex.getError().context());
+    assertEquals(Map.of("entity", "name", "provider", "my-provider"), ex.getError().context());
     assertEquals(500, ex.getStatusCode());
   }
 
+  @Test
+  @DisplayName("test getProvider: should resolve provider by type not by name")
+  void testGetProviderShouldResolveByType() {
+    var providerConfiguration = new ProviderConfiguration();
+    providerConfiguration.setName("my-http-provider");
+    providerConfiguration.setType("http");
+
+    var provider = Mockito.mock(ProviderPlugin.class);
+
+    Mockito.when(configurationService.getProviderConfiguration("my-http-provider"))
+        .thenReturn(Optional.of(providerConfiguration));
+    Mockito.when(providerFactory.getProviderByType("http")).thenReturn(Optional.of(provider));
+
+    var entityConfiguration = new EntityConfiguration();
+    entityConfiguration.setProvider("my-http-provider");
+    entityConfiguration.setName("users");
+    var entity = new DynamicEntity();
+    entity.setConfiguration(entityConfiguration);
+
+    var result = service.getProvider(entity);
+
+    assertEquals(provider, result);
+    // Verify that getProviderByType was called with the TYPE, not the provider NAME
+    Mockito.verify(providerFactory).getProviderByType("http");
+    Mockito.verify(providerFactory, Mockito.never()).getProviderByType("my-http-provider");
+  }
+
   @Test
   @DisplayName("test getProviderConfiguration: should throw exception without configuration")
   void testGetProviderConfiguration() {

src/test/java/io/github/linagora/linid/im/plugin/provider/ProviderFactoryImplTest.java

@@ -65,7 +65,7 @@ void testProviderFactoryImpl() {
     var provider = new SimpleProviderPlugin();
     Mockito.when(providerRegistry.getPlugins()).thenReturn(List.of(provider));
 
-    var result = providerFactory.getProviderByName("test");
+    var result = providerFactory.getProviderByType("test");
     assertNotNull(result);
     assertFalse(result.isEmpty());
     assertEquals(provider, result.get());
@@ -77,7 +77,7 @@ void testProviderFactoryImplEmpty() {
     var provider = new SimpleProviderPlugin();
     Mockito.when(providerRegistry.getPlugins()).thenReturn(List.of(provider));
 
-    var result = providerFactory.getProviderByName("bas");
+    var result = providerFactory.getProviderByType("bas");
     assertNotNull(result);
     assertTrue(result.isEmpty());
   }

src/test/java/io/github/linagora/linid/im/plugin/task/TaskEngineImplTest.java

@@ -140,10 +140,11 @@ void testGetPluginWithInvalidPlugin() {
   }
 
   @Test
-  @DisplayName("test getPlugin: should return plugin with valid plugin")
+  @DisplayName("test getPlugin: should return plugin matching by type")
   void testGetPluginWithValidPlugin() {
     TaskConfiguration config = new TaskConfiguration();
     config.setName("mytask");
+    config.setType("dummy-type");
 
     var plugin = new DummyPlugin();
     Mockito.when(taskRegistry.getPlugins()).thenReturn(List.of(plugin));
@@ -153,13 +154,29 @@ void testGetPluginWithValidPlugin() {
     assertEquals(plugin, result);
   }
 
+  @Test
+  @DisplayName("test getPlugin: should not match plugin by name when type differs")
+  void testGetPluginShouldMatchByTypeNotName() {
+    TaskConfiguration config = new TaskConfiguration();
+    config.setName("dummy-type"); // Name matches plugin type
+    config.setType("different-type"); // But type does not
+
+    var plugin = new DummyPlugin();
+    Mockito.when(taskRegistry.getPlugins()).thenReturn(List.of(plugin));
+
+    ApiException ex = assertThrows(ApiException.class, () -> taskEngine.getPlugin(config));
+    assertEquals("error.plugin.unknown", ex.getError().key());
+    assertEquals(Map.of("type", "different-type"), ex.getError().context());
+  }
+
   @Test
   @DisplayName("test execute: should execute plugin")
   void testExecutePlugin() {
     var plugin = Mockito.spy(new DummyPlugin());
 
     var config = new TaskConfiguration();
     config.setName("mytask");
+    config.setType("dummy-type");
     config.setPhases(List.of("myphase"));
 
     var entityConfig = new EntityConfiguration();
@@ -176,10 +193,27 @@ void testExecutePlugin() {
     Mockito.verify(plugin).execute(Mockito.any(), Mockito.eq(entity), Mockito.any());
   }
 
+  @Test
+  @DisplayName("test execute: should not throw when tasks list is null")
+  void testExecuteWithNullTasks() {
+    var entityConfig = new EntityConfiguration();
+    entityConfig.setTasks(null);
+
+    var entity = Mockito.mock(DynamicEntity.class);
+    Mockito.when(entity.getConfiguration()).thenReturn(entityConfig);
+
+    // Should not throw NullPointerException
+    taskEngine.execute(entity, new TaskExecutionContext(), "myphase");
+
+    // Verify no plugin interaction occurred
+    Mockito.verifyNoInteractions(taskRegistry);
+    Mockito.verifyNoInteractions(configurationService);
+  }
+
   public static class DummyPlugin implements TaskPlugin {
     @Override
-    public boolean supports(@NonNull String name) {
-      return "mytask".equals(name);
+    public boolean supports(@NonNull String type) {
+      return "dummy-type".equals(type);
     }
 
     @Override