This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch bugfix/service-values-in-scr-skill in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit b918f4661bfc5c2d369177495b5b174300eacc94 Author: Konrad Windszus <[email protected]> AuthorDate: Sat May 9 12:07:32 2026 +0200 Register all transitive interfaces as services as well --- .../scripts/migrate_annotations.py | 121 +++++++++- .../scripts/tests/test_migrate_annotations.py | 251 +++++++++++++++++++++ 2 files changed, 369 insertions(+), 3 deletions(-) diff --git a/skills/osgi-scr-migrator/scripts/migrate_annotations.py b/skills/osgi-scr-migrator/scripts/migrate_annotations.py index 70385722..6e87ee86 100755 --- a/skills/osgi-scr-migrator/scripts/migrate_annotations.py +++ b/skills/osgi-scr-migrator/scripts/migrate_annotations.py @@ -737,7 +737,10 @@ class AnnotationMigrator: return annotations def _find_service_class_for_component(self, component_idx: int) -> Optional[str]: - """Find the @Service annotation near this @Component.""" + """Find the @Service annotation and either infer service class from class interfaces or the given value. + + Checks for explicit @Service annotation. + """ # Look in the next few lines for @Service for i in range(component_idx + 1, min(component_idx + 5, len(self.lines))): line = self.lines[i] @@ -746,9 +749,121 @@ class AnnotationMigrator: service_match = re.search(r'@Service\s*\(\s*(?:value\s*=\s*)?([^)]+)\)', line) if service_match: return service_match.group(1).strip() - # If no explicit class, return empty to signal service registration - return "" + # If @Service is present but no class specified, we will infer from interfaces + else: + return self._get_transitive_interfaces_from_class(component_idx) + + # No @Service annotation found don't auto register as service return None + + def _get_transitive_interfaces_from_class(self, component_idx: int) -> Optional[str]: + """Extract all transitive interfaces from the class declaration. + + Looks for the class declaration after the @Component annotation and extracts + all interfaces it implements. Also resolves transitive interfaces (interfaces + that the declared interfaces extend). + + Returns: + - A single interface name if exactly one interface is found + - A list of interfaces formatted as "{ Interface1.class, Interface2.class, ... }" + - Empty string "" if interfaces are found but unable to resolve + - None if no class declaration or interfaces are found + """ + # Find the class declaration after @Component + class_line = None + for i in range(component_idx + 1, min(component_idx + 10, len(self.lines))): + line = self.lines[i] + if re.search(r'(public|private|protected)?\s*(abstract\s+)?class\s+\w+', line): + class_line = i + break + + if class_line is None: + return None + + # Extract the class declaration line(s) - may span multiple lines + class_decl = self.lines[class_line] + paren_count = class_decl.count('(') - class_decl.count(')') + j = class_line + 1 + + # Collect until we find opening brace or complete the declaration + while j < len(self.lines) and paren_count >= 0 and '{' not in class_decl: + class_decl += ' ' + self.lines[j].strip() + paren_count += self.lines[j].count('(') - self.lines[j].count(')') + j += 1 + + # Extract interfaces from "implements Interface1, Interface2, ..." + interfaces_match = re.search(r'implements\s+([^{]+)', class_decl) + if not interfaces_match: + return None + + interfaces_str = interfaces_match.group(1).strip() + # Split by comma and clean up each interface + declared_interfaces = [iface.strip() for iface in interfaces_str.split(',')] + + if not declared_interfaces: + return None + + # Get all transitive interfaces + all_interfaces = set() + for iface in declared_interfaces: + # Add the direct interface + all_interfaces.add(iface) + # Get transitive interfaces + transitive = self._get_transitive_extends(iface) + all_interfaces.update(transitive) + + # Remove any empty strings + all_interfaces.discard('') + + if not all_interfaces: + return None + + # Format the result + if len(all_interfaces) == 1: + return list(all_interfaces)[0] + ".class" + else: + # Sort for consistent output + sorted_interfaces = sorted(all_interfaces) + interfaces_expr = ', '.join(f'{iface}.class' for iface in sorted_interfaces) + return f'{{ {interfaces_expr} }}' + + def _get_transitive_extends(self, interface_name: str) -> Set[str]: + """Find all interfaces that the given interface extends (recursively). + + Searches the current file for the interface definition and extracts all + interfaces it extends, then recursively resolves those. + + Args: + interface_name: The interface name to resolve (may include package prefix) + + Returns: + Set of all transitive parent interfaces + """ + transitive = set() + + # Handle simple names (just look for "interface InterfaceName") + # and qualified names (com.example.InterfaceName - take last part) + simple_name = interface_name.split('.')[-1] if '.' in interface_name else interface_name + + # Search for interface definition in current file + for i, line in enumerate(self.lines): + # Look for "interface InterfaceName extends ..." + pattern = rf'interface\s+{re.escape(simple_name)}\s*(?:extends\s+([^{{]+))?' + match = re.search(pattern, line) + if match and match.group(1): + # Found the extends clause + extends_str = match.group(1).strip() + # Split by comma to get all parent interfaces + parent_interfaces = [iface.strip() for iface in extends_str.split(',')] + + for parent in parent_interfaces: + if parent: + transitive.add(parent) + # Recursively get transitive interfaces + transitive.update(self._get_transitive_extends(parent)) + break + + return transitive def _transform_component_annotation(self, annotation: str, service_class: Optional[str] = None) -> str: """Transform SCR @Component to OSGi R6/R7 format.""" diff --git a/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py b/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py index f15eb00d..ee476b95 100644 --- a/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py +++ b/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py @@ -708,6 +708,257 @@ public class MyService { self.assertIn('service_ranking()', new_content) +class TestTransitiveInterfaceResolution(unittest.TestCase): + """Test transitive interface retrieval from class declarations.""" + + def test_single_interface_implementation(self): + """Test retrieving a single interface from class declaration.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +public interface MyService { + void doSomething(); +} + +@Component +public class MyServiceImpl implements MyService { + public void doSomething() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + # Index of @Component annotation + result = migrator._get_transitive_interfaces_from_class(6) + + self.assertIsNotNone(result) + self.assertIn('MyService.class', result) + + def test_multiple_interfaces_implementation(self): + """Test retrieving multiple interfaces from class declaration.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +public interface Service1 { void method1(); } +public interface Service2 { void method2(); } + +@Component +public class MultiImpl implements Service1, Service2 { + public void method1() {} + public void method2() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(6) + + self.assertIsNotNone(result) + # Result should contain both interfaces + self.assertIn('Service1', result) + self.assertIn('Service2', result) + self.assertIn('.class', result) + + def test_transitive_extends_hierarchy(self): + """Test retrieving interfaces with transitive extends hierarchy.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +public interface BaseService { + void base(); +} + +public interface ExtendedService extends BaseService { + void extended(); +} + +@Component +public class ServiceImpl implements ExtendedService { + public void extended() {} + public void base() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(9) + + self.assertIsNotNone(result) + # Should include both the direct interface and transitive parent + self.assertIn('BaseService', result) + self.assertIn('ExtendedService', result) + + def test_complex_transitive_hierarchy(self): + """Test complex transitive interface hierarchy (A -> B -> C).""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +public interface A { void a(); } +public interface B extends A { void b(); } +public interface C extends B { void c(); } + +@Component +public class ComplexImpl implements C { + public void c() {} + public void b() {} + public void a() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(7) + + self.assertIsNotNone(result) + # Should include all three interfaces in the hierarchy + self.assertIn('A', result) + self.assertIn('B', result) + self.assertIn('C', result) + + def test_service_annotation_takes_precedence(self): + """Test that explicit @Service annotation takes precedence over class interfaces.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Service; + +public interface MyInterface { + void doIt(); +} + +@Component +@Service(ExplicitService.class) +public class MyImpl implements MyInterface { + public void doIt() {} +} + +public interface ExplicitService { + void explicit(); +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._find_service_class_for_component(8) + + # @Service annotation should take precedence + self.assertEqual(result, 'ExplicitService.class') + + def test_no_interfaces_returns_none(self): + """Test that components with no interfaces return None.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +@Component +public class PlainComponent { + public void doSomething() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(3) + + self.assertIsNone(result) + + def test_service_annotation_without_value(self): + """Test @Service annotation without explicit class value.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Service; + +public interface MyInterface { + void doIt(); +} + +@Component +@Service +public class MyImpl implements MyInterface { + public void doIt() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._find_service_class_for_component(8) + + # Should return "MyInterface" for @Service without value as the semantics of SCR Service and the service attribute in OSGi R6/R7 @Component are different for empty values. + self.assertEqual(result, "MyInterface.class") + + def test_multiline_class_declaration(self): + """Test retrieving interfaces from multi-line class declaration.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +public interface Service1 { void m1(); } +public interface Service2 { void m2(); } + +@Component +public class MultilineImpl + implements Service1, Service2 { + + public void m1() {} + public void m2() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(7) + + self.assertIsNotNone(result) + self.assertIn('Service1', result) + self.assertIn('Service2', result) + + def test_multiple_interface_extends(self): + """Test interfaces that extend multiple parent interfaces.""" + content = '''package com.example; + +import org.apache.felix.scr.annotations.Component; + +public interface A { void a(); } +public interface B { void b(); } +public interface C extends A, B { void c(); } + +@Component +public class MultiExtendImpl implements C { + public void c() {} + public void a() {} + public void b() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(7) + + self.assertIsNotNone(result) + # Should resolve all transitive parents + self.assertIn('A', result) + self.assertIn('B', result) + self.assertIn('C', result) + + def test_fully_qualified_interface_names(self): + """Test that interfaces are properly resolved even with package names.""" + content = '''package com.example.impl; + +import org.apache.felix.scr.annotations.Component; +import com.example.api.MyService; +import com.example.api.OtherService; + +@Component +public class ServiceImpl implements MyService, OtherService { + public void doIt() {} +} +''' + stats = MigrationStats() + migrator = AnnotationMigrator(content, Path("test.java"), stats) + result = migrator._get_transitive_interfaces_from_class(5) + + self.assertIsNotNone(result) + # Should contain the interface names (last part after package) + self.assertIn('MyService', result) + self.assertIn('OtherService', result) + + class TestMigrationStats(unittest.TestCase): """Test migration statistics tracking."""
