You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/12/02 07:44:32 UTC
[GitHub] [sling-org-apache-sling-models-jacksonexporter] kwin commented on a change in pull request #4: SLING-10959 Sling Models Exporter: Migrate from Felix SCR annotations to OSGi R7 annotations, Update to Parent 46
kwin commented on a change in pull request #4:
URL: https://github.com/apache/sling-org-apache-sling-models-jacksonexporter/pull/4#discussion_r760826356
##########
File path: pom.xml
##########
@@ -29,50 +36,50 @@
<developerConnection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-jacksonexporter.git</developerConnection>
<url>https://gitbox.apache.org/repos/asf?p=sling-org-apache-sling-models-jacksonexporter.git</url>
</scm>
- <build>
- <plugins>
- <plugin>
- <groupId>org.apache.felix</groupId>
- <artifactId>maven-scr-plugin</artifactId>
- </plugin>
- <plugin>
- <groupId>org.apache.felix</groupId>
- <artifactId>maven-bundle-plugin</artifactId>
- <extensions>true</extensions>
- <configuration>
- <instructions>
- <Embed-Dependency>*;scope=compile</Embed-Dependency>
- <Conditional-Package>org.apache.sling.commons.osgi</Conditional-Package>
- </instructions>
- </configuration>
- </plugin>
- </plugins>
- </build>
+
+ <properties>
+ <sling.java.version>8</sling.java.version>
+ <project.build.outputTimestamp>2021-12-01T00:00:00Z</project.build.outputTimestamp>
+ </properties>
+
<dependencies>
<dependency>
- <groupId>org.apache.sling</groupId>
- <artifactId>org.apache.sling.models.api</artifactId>
- <version>1.3.0</version>
+ <groupId>org.osgi</groupId>
+ <artifactId>osgi.core</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.osgi</groupId>
- <artifactId>org.osgi.compendium</artifactId>
+ <artifactId>osgi.cmpn</artifactId>
Review comment:
Let us rather use individual OSGi dependencies instead of aggregates.
##########
File path: pom.xml
##########
@@ -29,50 +36,50 @@
<developerConnection>scm:git:https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-jacksonexporter.git</developerConnection>
<url>https://gitbox.apache.org/repos/asf?p=sling-org-apache-sling-models-jacksonexporter.git</url>
</scm>
- <build>
- <plugins>
- <plugin>
- <groupId>org.apache.felix</groupId>
- <artifactId>maven-scr-plugin</artifactId>
- </plugin>
- <plugin>
- <groupId>org.apache.felix</groupId>
- <artifactId>maven-bundle-plugin</artifactId>
- <extensions>true</extensions>
- <configuration>
- <instructions>
- <Embed-Dependency>*;scope=compile</Embed-Dependency>
- <Conditional-Package>org.apache.sling.commons.osgi</Conditional-Package>
- </instructions>
- </configuration>
- </plugin>
- </plugins>
- </build>
+
+ <properties>
+ <sling.java.version>8</sling.java.version>
+ <project.build.outputTimestamp>2021-12-01T00:00:00Z</project.build.outputTimestamp>
+ </properties>
+
<dependencies>
<dependency>
- <groupId>org.apache.sling</groupId>
- <artifactId>org.apache.sling.models.api</artifactId>
- <version>1.3.0</version>
+ <groupId>org.osgi</groupId>
+ <artifactId>osgi.core</artifactId>
Review comment:
Use individual OSGi dependencies
##########
File path: src/main/java/org/apache/sling/models/jacksonexporter/impl/JacksonExporter.java
##########
@@ -58,9 +56,7 @@
private static final int MAPPER_FEATURE_PREFIX_LENGTH = MAPPER_FEATURE_PREFIX.length();
- @Reference(name = "moduleProvider", referenceInterface = ModuleProvider.class,
- cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE, policy = ReferencePolicy.DYNAMIC)
- private final RankedServices<ModuleProvider> moduleProviders = new RankedServices<ModuleProvider>(Order.ASCENDING);
+ private final RankedServices<ModuleProvider> moduleProviders = new RankedServices<>(Order.ASCENDING);
Review comment:
RankedServices are no longer necessary. Field injections are ordered by DS automatically: http://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#d0e37828
> SCR must set the field value with a new mutable collection that must contain the initial set of bound services sorted using the same ordering as ServiceReference.compareTo based upon service ranking and service id.
##########
File path: src/main/java/org/apache/sling/models/jacksonexporter/impl/JacksonExporter.java
##########
@@ -124,6 +120,9 @@ public boolean isSupported(@NotNull Class<?> clazz) {
}
}
+ @Reference(name = "moduleProvider", service = ModuleProvider.class,
+ cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC,
+ bind = "bindModuleProvider", unbind = "unbindModuleProvider")
protected void bindModuleProvider(final ModuleProvider moduleProvider, final Map<String, Object> props) {
Review comment:
Let us rely on field injection here.
##########
File path: src/main/java/org/apache/sling/models/jacksonexporter/impl/RequestModuleProvider.java
##########
@@ -21,19 +21,16 @@
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
-import org.apache.felix.scr.annotations.Component;
-import org.apache.felix.scr.annotations.Property;
-import org.apache.felix.scr.annotations.Service;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.models.jacksonexporter.ModuleProvider;
-import org.osgi.framework.Constants;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.propertytypes.ServiceRanking;
import com.fasterxml.jackson.databind.Module;
import com.fasterxml.jackson.databind.module.SimpleModule;
-@Component
-@Service
-@Property(name = Constants.SERVICE_RANKING, intValue = 0)
+@Component(service = ModuleProvider.class)
+@ServiceRanking(0)
Review comment:
As 0 is the default I am not sure we should explicitly set it here.
##########
File path: src/main/java/org/apache/sling/models/jacksonexporter/impl/JacksonExporter.java
##########
@@ -22,17 +22,16 @@
import java.io.StringWriter;
import java.util.Map;
-import org.apache.felix.scr.annotations.Component;
-import org.apache.felix.scr.annotations.Reference;
-import org.apache.felix.scr.annotations.ReferenceCardinality;
-import org.apache.felix.scr.annotations.ReferencePolicy;
-import org.apache.felix.scr.annotations.Service;
import org.apache.sling.commons.osgi.Order;
Review comment:
Let us try to get rid of all references to commons.osgi. Those are usually not necessary when relying on DS 1.4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org