karaf-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Estermann (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (KARAF-6183) FeaturesProcessorImpl improvement for bundle override
Date Wed, 06 Mar 2019 15:41:00 GMT

     [ https://issues.apache.org/jira/browse/KARAF-6183?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Daniel Estermann updated KARAF-6183:
------------------------------------
    Description: 
Say we have the following override.properties:

{{mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT;range="[0,99999)"}}
{{mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration;range="[0,99999)"}}

which are supposed to do the following replacements:

{{portal-backend-sdk/2.68.3}} → {{portal-backend-sdk/2.69.0-SNAPSHOT}}

and

{{portal-backend-sdk/2.68.3/jar/karaf-migration}} → {{portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration}}

But the method [FeaturesProcessorImpl.staticOverrideBundle(Bundle)|https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesProcessorImpl.java#L225]
replaces {{portal-backend-sdk/2.68.3/jar/karaf-migration}} with {{portal-backend-sdk/2.69.0-SNAPSHOT}},
i.e. a classified artifact gets replaced with an artifact without classifier. This happens
because the implementation of [LocationPattern|https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/LocationPattern.java#L151]
allows matching of classified URI by a non-classified pattern. The LocationPatternTest indicates
that this is an intentional behavior: see line [112|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L112],
[115|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L115],
[116|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L116].

I can understand why {{LocationPattern}} is implemented like that. But then my guess is that
{{FeaturesProcessorImpl}} is incorrect. There is a comment which shows the anticipation of
such misbehavior: {{TOCHECK: last rule wins - no break!!!}} Last rule indeed doesn't work
in this case. I think what is crucial here, is that we shouldn't rely on the order of overrides,
but rather on their quality. I.e. not the first/last match is appropriate, but the best one.
I propose to collect candidates to match and then determine the best one using simply the
length as criterion.

  was:
Say we have the following override.properties:

{{mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT;range="[0,99999)" mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration;range="[0,99999)"}}

which are supposed to do the following replacements:

{{portal-backend-sdk/2.68.3}} → {{portal-backend-sdk/2.69.0-SNAPSHOT}}

and

{{portal-backend-sdk/2.68.3/jar/karaf-migration}} → {{portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration}}

But the method [FeaturesProcessorImpl.staticOverrideBundle(Bundle)|https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesProcessorImpl.java#L225]
replaces {{portal-backend-sdk/2.68.3/jar/karaf-migration}} with {{portal-backend-sdk/2.69.0-SNAPSHOT}},
i.e. a classified artifact gets replaced with an artifact without classifier. This happens
because the implementation of [LocationPattern|https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/LocationPattern.java#L151]
allows matching of classified URI by a non-classified pattern. The LocationPatternTest indicates
that this is an intentional behavior: see line [112|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L112],
[115|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L115],
[116|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L116].

I can understand why {{LocationPattern}} is implemented like that. But then my guess is that
{{FeaturesProcessorImpl}} is incorrect. There is a comment which shows the anticipation of
such misbehavior: {{TOCHECK: last rule wins - no break!!!}} Last rule indeed doesn't work
in this case. I think what is crucial here, is that we shouldn't rely on the order of overrides,
but rather on their quality. I.e. not the first/last match is appropriate, but the best one.
I propose to collect candidates to match and then determine the best one using simply the
length as criterion.


> FeaturesProcessorImpl improvement for bundle override
> -----------------------------------------------------
>
>                 Key: KARAF-6183
>                 URL: https://issues.apache.org/jira/browse/KARAF-6183
>             Project: Karaf
>          Issue Type: Improvement
>          Components: karaf
>    Affects Versions: 4.2.3
>            Reporter: Daniel Estermann
>            Assignee: Jean-Baptiste Onofré
>            Priority: Major
>             Fix For: 4.3.0, 4.2.4
>
>
> Say we have the following override.properties:
> {{mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT;range="[0,99999)"}}
> {{mvn:com.seeburger.portal/portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration;range="[0,99999)"}}
> which are supposed to do the following replacements:
> {{portal-backend-sdk/2.68.3}} → {{portal-backend-sdk/2.69.0-SNAPSHOT}}
> and
> {{portal-backend-sdk/2.68.3/jar/karaf-migration}} → {{portal-backend-sdk/2.69.0-SNAPSHOT/jar/karaf-migration}}
> But the method [FeaturesProcessorImpl.staticOverrideBundle(Bundle)|https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesProcessorImpl.java#L225]
replaces {{portal-backend-sdk/2.68.3/jar/karaf-migration}} with {{portal-backend-sdk/2.69.0-SNAPSHOT}},
i.e. a classified artifact gets replaced with an artifact without classifier. This happens
because the implementation of [LocationPattern|https://github.com/apache/karaf/blob/master/features/core/src/main/java/org/apache/karaf/features/LocationPattern.java#L151]
allows matching of classified URI by a non-classified pattern. The LocationPatternTest indicates
that this is an intentional behavior: see line [112|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L112],
[115|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L115],
[116|https://github.com/apache/karaf/blob/master/features/core/src/test/java/org/apache/karaf/features/internal/service/LocationPatternTest.java#L116].
> I can understand why {{LocationPattern}} is implemented like that. But then my guess
is that {{FeaturesProcessorImpl}} is incorrect. There is a comment which shows the anticipation
of such misbehavior: {{TOCHECK: last rule wins - no break!!!}} Last rule indeed doesn't work
in this case. I think what is crucial here, is that we shouldn't rely on the order of overrides,
but rather on their quality. I.e. not the first/last match is appropriate, but the best one.
I propose to collect candidates to match and then determine the best one using simply the
length as criterion.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message