[jira] [Commented] (SLING-10030) Add an optional ranking as a property for PostOperation implementation

["Georg Henzler (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-10030?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17260345#comment-17260345 ] 

Georg Henzler commented on SLING-10030:
---------------------------------------

[~tomas.almeida] The current behaviour is certainly not ideal, a PR for this would be welcome. 

bq. bindPostOperation method will store all PostOperations by order of ranking (higher first), PostOperations with no ranking will be added to the end by order of calling.

If no ranking property is present we should follow the default [SRC semantics|https://docs.osgi.org/specification/osgi.cmpn/7.0.0/service.component.html#service.component-reference.cardinality] and assume 0. Also {{ReferencePolicyOption.GREEDY}} should be added to https://github.com/apache/sling-org-apache-sling-servlets-post/blob/c8b623f88ae9a2e087756172d6ad368a655b94f7/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java#L535 to ensure all service updates are reflected to the SlingPostServlet.

> Add an optional ranking as a property for PostOperation implementation 
> -----------------------------------------------------------------------
>
>                 Key: SLING-10030
>                 URL: https://issues.apache.org/jira/browse/SLING-10030
>             Project: Sling
>          Issue Type: Improvement
>    Affects Versions: Servlets Post 2.3.36
>            Reporter: Tomás Dias Almeida
>            Priority: Major
>
> Given the case, a PostOperation PO1 implementation is provided for a given operation, but it does not solve a particular need. I would like to be able to create another PostOperation implementation PO2 with a higher ranking and replace the PO1.
> h2. *Current status*
> h3. *Last wins condition*
> [org.apache.sling.servlets.post.impl.SlingPostServlet|https://github.com/apache/sling-org-apache-sling-servlets-post/blob/master/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java] can only store one PostOperation per operation as it is stored like
>  
> {code:java}
> private final Map<String, PostOperation> postOperations = new HashMap<>();{code}
>  
> This map is filled by the binding function per order of activation
>  
> {code:java}
> protected void bindPostOperation(final PostOperation operation, final Map<String, Object> properties) {
>     final String operationName = (String) properties.get(PostOperation.PROP_OPERATION_NAME);
>     if ( operationName != null && operation != null ) {
>         synchronized (this.postOperations) {
>             this.postOperations.put(operationName, operation);
>         }
>     }
> }{code}
>  
> If two classes provide an implementation for the given operation, we run into a concurrency problem and the last activated component "wins" the race and will be used.
> h3. Incorrect map if a component is stopped
> The map is updated using an unbinding method, that relies on the axiom that there is only one PostOperation per operation.
>  
> {code:java}
> protected void unbindPostOperation(final PostOperation operation, final Map<String, Object> properties) {
>     final String operationName = (String) properties.get(PostOperation.PROP_OPERATION_NAME);
>     if ( operationName != null ) {
>         synchronized (this.postOperations) {
>             this.postOperations.remove(operationName);
>         }
>     }
> }
> {code}
> And so, if we have the following scenario: 
>  
>  # PostOperation P1 is activated for operation P
>  # PostOperation P2 is activated for operation P
>  # [SlingPostServlet|https://github.com/apache/sling-org-apache-sling-servlets-post/blob/master/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java] stores P2 as PostOperation for P (as described above)
>  # P2 is stopped for any reason
>  # [SlingPostServlet|https://github.com/apache/sling-org-apache-sling-servlets-post/blob/master/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java] has no PostOperation set for P, but P1 could be the right one.
> h2. Proposal
> Each PostOperation can provide an optional "operation.ranking" integer property.
> bindPostOperation method will store all PostOperations by order of ranking (higher first), PostOperations with no ranking will be added to the end by order of calling.
> unbindPostOperation removes the given PostOperation of the sorted list.
>  
>  
> If you believe that is a good idea, I can provide an implementation to discuss the solution.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)