[jira] [Commented] (FELIX-6490) Files that cannot be deployed due to a missing handler are not processed again

["Robert Schulte (Jira)" <ji...@apache.org>]

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

Robert Schulte commented on FELIX-6490:
---------------------------------------

I can provide a PR with one of the suggested fixes if that helps

> Files that cannot be deployed due to a missing handler are not processed again
> ------------------------------------------------------------------------------
>
>                 Key: FELIX-6490
>                 URL: https://issues.apache.org/jira/browse/FELIX-6490
>             Project: Felix
>          Issue Type: Bug
>          Components: File Install
>    Affects Versions: fileinstall-3.7.0, fileinstall-3.7.2
>            Reporter: Robert Schulte
>            Priority: Major
>
> h2. Context
> I tried to migrate a custom Karaf distribution from 4.2.8 to 4.3.3 (fileinstall version 3.6.4 -> 3.7.0). The distribution contains a blueprint (xml) file that is supposed to be handled by one of Karaf's custom ArtifactListener_s. This kind of deploment works for Karaf 4.2.8 but fails for 4.3.3
> h2. Issue Analysis
> [DirectoryWatcher.java#doProcess|https://github.com/apache/felix-dev/blob/org.apache.felix.fileinstall-3.7.0/fileinstall/src/main/java/org/apache/felix/fileinstall/internal/DirectoryWatcher.java#L381-L385] has a mechanic of retrying deployment for files that could not be matched to a handler on a previous attempt:
> {code:java}
> private void doProcess(Set<File> files) throws InterruptedException
>     {
>         List<ArtifactListener> listeners = fileInstall.getListeners();
>         List<Artifact> deleted = new ArrayList<Artifact>();
>         List<Artifact> modified = new ArrayList<Artifact>();
>         List<Artifact> created = new ArrayList<Artifact>();
>         // Try to process again files that could not be processed
>         synchronized (processingFailures)
>         {
>             files.addAll(processingFailures);
>             processingFailures.clear();
>         }
> 		//...
> 	}
> {code}
> It is vital that doProcess is invoked although no changes have been observed in the watched folder. Otherwise, the retry mechanism is not effective at all.
> The workaround for FELIX-6229 ([101a360248311817e1ad4645c549ea77773b0481|https://github.com/apache/felix-dev/commit/101a360248311817e1ad4645c549ea77773b0481#diff-263cdbacfd163ef5ce31dcbb1db83138f78d88eab353f1f587a10199e8ef3817]) introduced a regression (in addition to the possible NPE that was fixed in cbf8174b66af21453ae209c590d4bf7f4a36e36b) that prevents doProcess from being invoked in the absence of changes in the watched directory.
> {code:java}
> if (files != null && !files.isEmpty()) {
>     process(files);
> }
> {code}
> The added guard {{!files.isEmpty()}} to {{process(files)}} renders the retrying logic ineffective
> h2. Possible fixes
> h3. Revert to old guard
> {code:java}
> if (files != null) {
>     process(files);
> }
> {code}
> h3. Account for "processingFailures"
> {code:java}
> if ((files != null && !files.isEmpty()) || !processingFailures.isEmpty()) {
>     process(files);
> }
> {code}
> This should probably be accompanied by some refactoring. If the re-processing of processingFailures was more apparent (e.g., by having a dedicated method for this),
>  * it becomes less likely, that the re-processing gets accidentally broken again
>  * re-processing could even be decoupled from the scanning interval



--
This message was sent by Atlassian Jira
(v8.20.1#820001)