This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
The following commit(s) were added to refs/heads/master by this push:
new 036d94dd Distinguish between literal and reference property values
036d94dd is described below
commit 036d94dd42260ee6cc719224098f0b497332683e
Author: Konrad Windszus <[email protected]>
AuthorDate: Sat May 9 11:51:05 2026 +0200
Distinguish between literal and reference property values
---
.../scripts/migrate_annotations.py | 218 +++++++++++----------
.../scripts/tests/test_migrate_annotations.py | 118 ++++++++++-
2 files changed, 230 insertions(+), 106 deletions(-)
diff --git a/skills/osgi-scr-migrator/scripts/migrate_annotations.py
b/skills/osgi-scr-migrator/scripts/migrate_annotations.py
index acccd3f4..31e9eb6c 100755
--- a/skills/osgi-scr-migrator/scripts/migrate_annotations.py
+++ b/skills/osgi-scr-migrator/scripts/migrate_annotations.py
@@ -41,6 +41,12 @@ class Property:
description: Optional[str] = None
is_configurable: bool = False
+ def _strip_quotes(self, val: str) -> str:
+ """Strip surrounding quotes from a value if present."""
+ if val.startswith('"') and val.endswith('"'):
+ return val[1:-1]
+ return val
+
def is_property_type_annotation(self) -> bool:
"""Check if this property should use a Component Property Type
annotation.
@@ -77,27 +83,64 @@ class Property:
Precondition: is_property_type_annotation() returned True.
This method should always succeed if the precondition is met.
+ Unquotes literal values before using them in annotations.
"""
+ val = self._strip_quotes(self.value)
+
if self.name == 'service.ranking':
- return f'@ServiceRanking({self.value})'
+ return f'@ServiceRanking({val})'
elif self.name == 'service.description':
- return f'@ServiceDescription("{self.value}")'
+ return f'@ServiceDescription("{val}")'
elif self.name == 'service.vendor':
- return f'@ServiceVendor("{self.value}")'
+ return f'@ServiceVendor("{val}")'
# Should never reach here if precondition is met
raise ValueError(f"Unknown property type annotation: {self.name}")
def to_component_property(self) -> List[str]:
- """Convert to @Component property format."""
+ """Convert to @Component property format.
+
+ Constant references are formatted as string concatenation:
"name="+CONSTANT
+ Literal values (with quotes stripped) are kept unquoted within the
property string: "name=value"
+ """
properties = []
type_suffix = f":{self.type.value}" if self.type !=
PropertyType.STRING else ""
+ def is_reference(val: str) -> bool:
+ """Return True when the value is not a valid literal for this
property type."""
+ val = val.strip()
+
+ if self.type == PropertyType.STRING:
+ return not val.startswith('"')
+
+ if self.type == PropertyType.BOOLEAN:
+ return not bool(re.fullmatch(r'true|false', val))
+
+ if self.type in (PropertyType.INTEGER, PropertyType.LONG):
+ return not bool(re.fullmatch(r'-?\d+[lL]?', val))
+
+ if self.type in (PropertyType.FLOAT, PropertyType.DOUBLE):
+ return not
bool(re.fullmatch(r'-?(?:\d+\.\d*|\d*\.\d+|\d+)(?:[fFdD])?', val))
+
+ return True
+
if self.values:
for val in self.values:
- properties.append(f'"{self.name}{type_suffix}={val}"')
+ if is_reference(val):
+ # Reference - use string concatenation: "name="+CONSTANT
+ properties.append(f'"{self.name}{type_suffix}="+{val}')
+ else:
+ # Literal value - strip quotes and keep unquoted within
property string
+ stripped_val = self._strip_quotes(val)
+
properties.append(f'"{self.name}{type_suffix}={stripped_val}"')
elif self.value is not None:
- properties.append(f'"{self.name}{type_suffix}={self.value}"')
+ if is_reference(self.value):
+ # Reference - use string concatenation: "name="+CONSTANT
+ properties.append(f'"{self.name}{type_suffix}="+{self.value}')
+ else:
+ # Literal value - strip quotes and keep unquoted within
property string
+ stripped_val = self._strip_quotes(self.value)
+ properties.append(f'"{self.name}{type_suffix}={stripped_val}"')
return properties
@@ -359,7 +402,10 @@ class AnnotationMigrator:
# Parse attributes
name_match = re.search(r'name\s*=\s*"([^"]*)"', annotation)
- value_match =
re.search(r'(?<!bool)(?<!int)(?<!long)(?<!float)(?<!double)value\s*=\s*"([^"]*)"',
annotation)
+ value_match = re.search(
+
r'(?<!bool)(?<!int)(?<!long)(?<!float)(?<!double)value\s*=\s*("[^"]*"|[A-Za-z_][A-Za-z0-9_.]*)',
+ annotation
+ )
values_match = re.search(r'value\s*=\s*\{([^}]+)\}', annotation)
bool_value_match = re.search(r'boolValue\s*=\s*(true|false)',
annotation)
int_value_match = re.search(r'intValue\s*=\s*(-?\d+)', annotation)
@@ -406,11 +452,13 @@ class AnnotationMigrator:
prop.type = PropertyType.DOUBLE
prop.value = double_value_match.group(1)
elif values_match:
- # Array of values
+ # Array of values - preserve quotes on string literals
values_str = values_match.group(1)
- prop.values = [v.strip().strip('"') for v in values_str.split(',')]
+ prop.values = [v.strip() for v in values_str.split(',')]
elif value_match:
- prop.value = value_match.group(1)
+ raw_value = value_match.group(1)
+ # Keep string literals quoted; keep constant references unquoted.
+ prop.value = raw_value
if label_match:
prop.label = label_match.group(1)
@@ -447,6 +495,25 @@ class AnnotationMigrator:
def _migrate_sling_servlet(self):
"""Migrate @SlingServlet to @Component with Sling Servlets
Annotations."""
+ def parse_annotation_values(annotation_text: str, attr_name: str) ->
Optional[List[str]]:
+ """Parse a SlingServlet attribute as a list of raw Java values.
+
+ Values are returned exactly as Java expressions (quoted string
literal or constant
+ reference), so callers can preserve constants without adding
quotes.
+ """
+ array_match = re.search(rf'{attr_name}\s*=\s*\{{([^}}]+)\}}',
annotation_text)
+ if array_match:
+ return [v.strip() for v in array_match.group(1).split(',') if
v.strip()]
+
+ single_match = re.search(
+ rf'{attr_name}\s*=\s*("[^"]+"|[A-Za-z_][A-Za-z0-9_.]*)',
+ annotation_text
+ )
+ if single_match:
+ return [single_match.group(1).strip()]
+
+ return None
+
i = 0
while i < len(self.lines):
line = self.lines[i]
@@ -469,12 +536,8 @@ class AnnotationMigrator:
indent_str = ' ' * indent
# Determine if path-based or resource-type-based
- paths_match = re.search(r'paths\s*=\s*\{([^}]+)\}', annotation)
- path_match = re.search(r'paths\s*=\s*"([^"]+)"', annotation)
if not paths_match else None
- rt_match = re.search(r'resourceTypes\s*=\s*\{([^}]+)\}',
annotation)
- rt_single_match = re.search(r'resourceTypes\s*=\s*"([^"]+)"',
annotation) if not rt_match else None
- # Also match constant references like
ActivityManagerImpl.RESOURCE_TYPE
- rt_constant_match =
re.search(r'resourceTypes\s*=\s*([A-Za-z_][A-Za-z0-9_.]*)', annotation) if not
(rt_match or rt_single_match) else None
+ path_values = parse_annotation_values(annotation, 'paths')
+ rt_values = parse_annotation_values(annotation,
'resourceTypes')
new_annotations = []
@@ -482,86 +545,29 @@ class AnnotationMigrator:
new_annotations.append(f'{indent_str}@Component(service =
Servlet.class)')
# Path-based servlet
- if paths_match or path_match:
- if paths_match:
- paths = [p.strip().strip('"') for p in
paths_match.group(1).split(',')]
- paths_str = ', '.join(f'"{p}"' for p in paths)
+ if path_values:
+ if len(path_values) == 1:
+
new_annotations.append(f'{indent_str}@SlingServletPaths(value =
{path_values[0]})')
+ else:
+ paths_str = ', '.join(path_values)
new_annotations.append(f'{indent_str}@SlingServletPaths(value =
{{{paths_str}}})')
- elif path_match:
-
new_annotations.append(f'{indent_str}@SlingServletPaths(value =
"{path_match.group(1)}")')
# Resource-type-based servlet
- elif rt_match or rt_single_match or rt_constant_match:
+ elif rt_values:
attrs = []
- if rt_match:
- # Parse array elements, preserving whether they're
string literals or constants
- raw_rts = [rt.strip() for rt in
rt_match.group(1).split(',')]
- formatted_rts = []
- for rt in raw_rts:
- if rt.startswith('"') and rt.endswith('"'):
- # String literal - keep as is
- formatted_rts.append(rt)
- else:
- # Constant reference - keep without quotes
- formatted_rts.append(rt)
-
- if len(formatted_rts) == 1:
- # Single element - check if it needs quotes
- if formatted_rts[0].startswith('"'):
- attrs.append(f'resourceTypes =
{formatted_rts[0]}')
- else:
- attrs.append(f'resourceTypes =
{formatted_rts[0]}')
+ def append_attr(attr_name: str, values:
Optional[List[str]]):
+ if not values:
+ return
+ if len(values) == 1:
+ attrs.append(f'{attr_name} = {values[0]}')
else:
- rts_str = ', '.join(formatted_rts)
- attrs.append(f'resourceTypes = {{{rts_str}}}')
- elif rt_single_match:
- attrs.append(f'resourceTypes =
"{rt_single_match.group(1)}"')
- elif rt_constant_match:
- # Constant reference - use as-is without quotes
- attrs.append(f'resourceTypes =
{rt_constant_match.group(1)}')
-
- # Selectors
- selectors_match =
re.search(r'selectors\s*=\s*\{([^}]+)\}', annotation)
- selector_match = re.search(r'selectors\s*=\s*"([^"]+)"',
annotation) if not selectors_match else None
-
- if selectors_match:
- sels = [s.strip().strip('"') for s in
selectors_match.group(1).split(',')]
- if len(sels) == 1:
- attrs.append(f'selectors = "{sels[0]}"')
- else:
- sels_str = ', '.join(f'"{s}"' for s in sels)
- attrs.append(f'selectors = {{{sels_str}}}')
- elif selector_match:
- attrs.append(f'selectors =
"{selector_match.group(1)}"')
-
- # Extensions
- ext_match = re.search(r'extensions\s*=\s*\{([^}]+)\}',
annotation)
- ext_single_match =
re.search(r'extensions\s*=\s*"([^"]+)"', annotation) if not ext_match else None
-
- if ext_match:
- exts = [e.strip().strip('"') for e in
ext_match.group(1).split(',')]
- if len(exts) == 1:
- attrs.append(f'extensions = "{exts[0]}"')
- else:
- exts_str = ', '.join(f'"{e}"' for e in exts)
- attrs.append(f'extensions = {{{exts_str}}}')
- elif ext_single_match:
- attrs.append(f'extensions =
"{ext_single_match.group(1)}"')
-
- # Methods
- methods_match = re.search(r'methods\s*=\s*\{([^}]+)\}',
annotation)
- method_match = re.search(r'methods\s*=\s*"([^"]+)"',
annotation) if not methods_match else None
-
- if methods_match:
- methods = [m.strip().strip('"') for m in
methods_match.group(1).split(',')]
- if len(methods) == 1:
- attrs.append(f'methods = "{methods[0]}"')
- else:
- methods_str = ', '.join(f'"{m}"' for m in methods)
- attrs.append(f'methods = {{{methods_str}}}')
- elif method_match:
- attrs.append(f'methods = "{method_match.group(1)}"')
+ attrs.append(f'{attr_name} = {{{",
".join(values)}}}')
+
+ append_attr('resourceTypes', rt_values)
+ append_attr('selectors',
parse_annotation_values(annotation, 'selectors'))
+ append_attr('extensions',
parse_annotation_values(annotation, 'extensions'))
+ append_attr('methods', parse_annotation_values(annotation,
'methods'))
# Build @SlingServletResourceTypes annotation
if len(attrs) == 1:
@@ -585,7 +591,7 @@ class AnnotationMigrator:
# Add imports
self.imports_to_add.add('org.osgi.service.component.annotations.Component')
self.imports_to_add.add('javax.servlet.Servlet')
- if paths_match or path_match:
+ if path_values:
self.imports_to_add.add('org.apache.sling.servlets.annotations.SlingServletPaths')
else:
self.imports_to_add.add('org.apache.sling.servlets.annotations.SlingServletResourceTypes')
@@ -599,6 +605,14 @@ class AnnotationMigrator:
def _migrate_sling_filter(self):
"""Migrate @SlingFilter to @Component with @SlingServletFilter
annotation."""
+ def parse_single_annotation_value(annotation_text: str, attr_name:
str) -> Optional[str]:
+ """Parse a SlingFilter single-value attribute as raw Java
expression."""
+ match = re.search(
+ rf'{attr_name}\s*=\s*("[^"]+"|-?\d+|[A-Za-z_][A-Za-z0-9_.]*)',
+ annotation_text
+ )
+ return match.group(1).strip() if match else None
+
i = 0
while i < len(self.lines):
line = self.lines[i]
@@ -623,19 +637,21 @@ class AnnotationMigrator:
attrs = []
# Scope
- scope_match =
re.search(r'scope\s*=\s*SlingFilterScope\.(\w+)', annotation)
- if scope_match:
- attrs.append(f'scope =
SlingServletFilterScope.{scope_match.group(1)}')
+ scope_value = parse_single_annotation_value(annotation,
'scope')
+ if scope_value:
+ # Map old enum constants to the new enum class, preserve
custom constants as-is.
+ scope_value = scope_value.replace('SlingFilterScope.',
'SlingServletFilterScope.')
+ attrs.append(f'scope = {scope_value}')
# Order (maps to order parameter in @SlingServletFilter)
- order_match = re.search(r'order\s*=\s*(-?\d+)', annotation)
- if order_match:
- attrs.append(f'order = {order_match.group(1)}')
+ order_value = parse_single_annotation_value(annotation,
'order')
+ if order_value:
+ attrs.append(f'order = {order_value}')
# Pattern
- pattern_match = re.search(r'pattern\s*=\s*"([^"]+)"',
annotation)
- if pattern_match:
- attrs.append(f'pattern = "{pattern_match.group(1)}"')
+ pattern_value = parse_single_annotation_value(annotation,
'pattern')
+ if pattern_value:
+ attrs.append(f'pattern = {pattern_value}')
# Generate annotations
new_annotations = []
@@ -666,7 +682,7 @@ class AnnotationMigrator:
self.imports_to_add.add('org.osgi.service.component.annotations.Component')
self.imports_to_add.add('javax.servlet.Filter')
self.imports_to_add.add('org.apache.sling.servlets.annotations.SlingServletFilter')
- if scope_match:
+ if scope_value and ('SlingServletFilterScope.' in scope_value):
self.imports_to_add.add('org.apache.sling.servlets.annotations.SlingServletFilterScope')
self.changes_made = True
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 23f39628..598d8458 100644
--- a/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py
+++ b/skills/osgi-scr-migrator/scripts/tests/test_migrate_annotations.py
@@ -23,7 +23,7 @@ from migrate_annotations import (
class TestPropertyParsing(unittest.TestCase):
"""Test @Property annotation parsing."""
- def test_simple_string_property(self):
+ def test_simple_string_literal_property(self):
"""Test parsing simple string property."""
content = '''
@Property(name = "service.vendor", value = "Apache Software Foundation")
@@ -35,7 +35,22 @@ class TestPropertyParsing(unittest.TestCase):
self.assertEqual(len(migrator.component_properties), 1)
prop = migrator.component_properties[0]
self.assertEqual(prop.name, "service.vendor")
- self.assertEqual(prop.value, "Apache Software Foundation")
+ self.assertEqual(prop.value, "\"Apache Software Foundation\"")
+ self.assertEqual(prop.type, PropertyType.STRING)
+
+ def test_simple_string_reference_property(self):
+ """Test parsing simple string property."""
+ content = '''
+@Property(name = "service.vendor", value = Constants.SERVICE_VENDOR)
+'''
+ stats = MigrationStats()
+ migrator = AnnotationMigrator(content, Path("test.java"), stats)
+ migrator._collect_properties()
+
+ self.assertEqual(len(migrator.component_properties), 1)
+ prop = migrator.component_properties[0]
+ self.assertEqual(prop.name, "service.vendor")
+ self.assertEqual(prop.value, "Constants.SERVICE_VENDOR")
self.assertEqual(prop.type, PropertyType.STRING)
def test_boolean_property(self):
@@ -81,7 +96,7 @@ class TestPropertyParsing(unittest.TestCase):
prop = migrator.component_properties[0]
self.assertEqual(prop.name, "paths")
self.assertEqual(len(prop.values), 3)
- self.assertEqual(prop.values, ["/path1", "/path2", "/path3"])
+ self.assertEqual(prop.values, ["\"/path1\"", "\"/path2\"",
"\"/path3\""])
def test_property_with_label(self):
"""Test parsing property with label and description."""
@@ -105,7 +120,7 @@ class TestPropertyConversion(unittest.TestCase):
def test_simple_property_conversion(self):
"""Test converting simple property to component format."""
- prop = Property(name="service.vendor", value="Apache Software
Foundation")
+ prop = Property(name="service.vendor", value="\"Apache Software
Foundation\"")
result = prop.to_component_property()
self.assertEqual(len(result), 1)
@@ -129,7 +144,7 @@ class TestPropertyConversion(unittest.TestCase):
def test_array_property_conversion(self):
"""Test converting array property."""
- prop = Property(name="paths", values=["/path1", "/path2"])
+ prop = Property(name="paths", values=["\"/path1\"", "\"/path2\""])
result = prop.to_component_property()
self.assertEqual(len(result), 2)
@@ -219,6 +234,50 @@ public class MyServlet extends SlingAllMethodsServlet {
self.assertNotIn('org.apache.felix.scr.annotations.sling.SlingServlet',
new_content)
self.assertIn('org.apache.sling.servlets.annotations.SlingServletResourceTypes',
new_content)
+ def test_servlet_with_constant_values_for_all_resource_type_elements(self):
+ """Test constants are preserved for
resourceTypes/selectors/extensions/methods."""
+ content = '''package com.example;
+
+import org.apache.felix.scr.annotations.sling.SlingServlet;
+
+@SlingServlet(
+ resourceTypes = MyServlet.RESOURCE_TYPE,
+ selectors = {MyServlet.SELECTOR_A, "print"},
+ extensions = {MyServlet.EXT_JSON, "html"},
+ methods = {HttpConstants.METHOD_GET, "POST"}
+)
+public class MyServlet extends SlingAllMethodsServlet {
+}
+'''
+ stats = MigrationStats()
+ migrator = AnnotationMigrator(content, Path("test.java"), stats)
+ new_content, changed = migrator.migrate()
+
+ self.assertTrue(changed)
+ self.assertIn('@SlingServletResourceTypes', new_content)
+ self.assertIn('resourceTypes = MyServlet.RESOURCE_TYPE', new_content)
+ self.assertIn('selectors = {MyServlet.SELECTOR_A, "print"}',
new_content)
+ self.assertIn('extensions = {MyServlet.EXT_JSON, "html"}', new_content)
+ self.assertIn('methods = {HttpConstants.METHOD_GET, "POST"}',
new_content)
+
+ def test_path_based_servlet_with_constant_paths(self):
+ """Test constants are preserved for paths attribute."""
+ content = '''package com.example;
+
+import org.apache.felix.scr.annotations.sling.SlingServlet;
+
+@SlingServlet(paths = {MyServlet.PATH_A, "/bin/fallback"})
+public class MyServlet extends SlingAllMethodsServlet {
+}
+'''
+ stats = MigrationStats()
+ migrator = AnnotationMigrator(content, Path("test.java"), stats)
+ new_content, changed = migrator.migrate()
+
+ self.assertTrue(changed)
+ self.assertIn('@SlingServletPaths(value = {MyServlet.PATH_A,
"/bin/fallback"})', new_content)
+
self.assertIn('org.apache.sling.servlets.annotations.SlingServletPaths',
new_content)
+
def test_sling_filter_migration(self):
"""Test migrating Sling filter to Sling Servlets Annotations."""
content = '''package com.example;
@@ -248,6 +307,55 @@ public class MyFilter implements Filter {
self.assertIn('org.apache.sling.servlets.annotations.SlingServletFilter',
new_content)
self.assertIn('org.apache.sling.servlets.annotations.SlingServletFilterScope',
new_content)
+ def test_sling_filter_with_constant_values(self):
+ """Test constants are preserved for SlingFilter attributes."""
+ content = '''package com.example;
+
+import org.apache.felix.scr.annotations.sling.SlingFilter;
+
+@SlingFilter(
+ scope = MyFilter.SCOPE,
+ order = MyFilter.ORDER,
+ pattern = MyFilter.PATTERN
+)
+public class MyFilter implements Filter {
+}
+'''
+ stats = MigrationStats()
+ migrator = AnnotationMigrator(content, Path("test.java"), stats)
+ new_content, changed = migrator.migrate()
+
+ self.assertTrue(changed)
+ self.assertIn('@SlingServletFilter', new_content)
+ self.assertIn('scope = MyFilter.SCOPE', new_content)
+ self.assertIn('order = MyFilter.ORDER', new_content)
+ self.assertIn('pattern = MyFilter.PATTERN', new_content)
+
+ def test_sling_filter_scope_enum_mapping_with_constants(self):
+ """Test old SlingFilterScope enum constants are mapped to
SlingServletFilterScope."""
+ content = '''package com.example;
+
+import org.apache.felix.scr.annotations.sling.SlingFilter;
+import org.apache.felix.scr.annotations.sling.SlingFilterScope;
+
+@SlingFilter(
+ scope = SlingFilterScope.INCLUDE,
+ order = FilterOrder.ORDER,
+ pattern = FilterPatterns.REQUEST
+)
+public class MyFilter implements Filter {
+}
+'''
+ stats = MigrationStats()
+ migrator = AnnotationMigrator(content, Path("test.java"), stats)
+ new_content, changed = migrator.migrate()
+
+ self.assertTrue(changed)
+ self.assertIn('scope = SlingServletFilterScope.INCLUDE', new_content)
+ self.assertIn('order = FilterOrder.ORDER', new_content)
+ self.assertIn('pattern = FilterPatterns.REQUEST', new_content)
+
self.assertIn('org.apache.sling.servlets.annotations.SlingServletFilterScope',
new_content)
+
class TestComponentMigration(unittest.TestCase):
"""Test @Component migration."""
