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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
