[jira] [Reopened] (FELIX-4548) Implement the missing errors registration
[ https://issues.apache.org/jira/browse/FELIX-4548?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Carsten Ziegeler reopened FELIX-4548: - Implement the missing errors registration - Key: FELIX-4548 URL: https://issues.apache.org/jira/browse/FELIX-4548 Project: Felix Issue Type: Sub-task Components: HTTP Service Affects Versions: http-2.4.0 Reporter: Simone Tripodi Assignee: Carsten Ziegeler Fix For: http.base-3.0.0 Attachments: FELIX-4548.patch, felix-4548-exception-hierarchy.patch Current implementation of RFC189 does not support yet errors registration. Questions are: * do we want to have a servlet (or maybe a filter fits better) implementation which serves a static file whenever an HTTP error is detected? or * looking for a solution that delegates the underlying servlet container? -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (FELIX-4848) Split ResolverImpl
[ https://issues.apache.org/jira/browse/FELIX-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14488112#comment-14488112 ] Christian Schneider commented on FELIX-4848: I did not mention any rule but 2300 lines is far beyond any limits I have seen recently. Of course you are right that simply splitting a class in half would not work if then the two classes are highly entangled. I checked the dependencies before proposing the split. ResolverImpl would have 3 calls to ConsistencyCheck and there would be no calls in the other direction. So it would be possible to understand ConsistencyCheck individually. Besides that the scope is also nicely described by the class name. Split ResolverImpl -- Key: FELIX-4848 URL: https://issues.apache.org/jira/browse/FELIX-4848 Project: Felix Issue Type: Improvement Components: Resolver Affects Versions: resolver-1.0.0 Reporter: Christian Schneider Fix For: resolver-1.2.0 ResolverImpl currently contains about 2300 lines of code. That is way too big for a single class. I looked into it and found that the checkDynamicPackageSpaceConsistency and checkPackageSpaceConsistency methods and their dependent methods form a nice subset. I would move that into a class ConsistencyCheck. Both would share all of the inner classes of ResolverImpl but nothing else. So I think i would make sense to move these inner classes to separate files. These changes should nicely split the classes into ResolverImpl : 1400 lines ConsistencyCheck : 600 lines -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (FELIX-4847) Make TemporalServiceDependency always available (and/or allow it to be optional)
[
https://issues.apache.org/jira/browse/FELIX-4847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14487040#comment-14487040
]
Tuomas Kiviaho commented on FELIX-4847:
---
In my case the temporal service remained unavailable (used dm shell to verify
this) until the actual service arrived. This is the behavior expected from
{{required}} temporal service dependency and I see now that tampering with what
{{isAvailable()}} returns - as per 1) proposal - would break the backwards
compatibility. I did not understand that the intention was to actually wait
until there is a service before dependency becomes available.
Therefore my only working proposal is 2) that is allowing the temporal service
to be optional as well. What I'd like to happen in this mode is for the
dependency to be available immediately and for the proxy to block/wait for the
services first appearance if user happens to access it prematurely. This is
indeed what would happen if optionality would not have been explicitly
forbidden in {{setRequired}} method. DefaultNullObject used by non-temporal
service dependency instead would return null immediately causing NPE in my
usecase.
This is a snippet from my PDE JUnit test case (magic happens inside a rule
below that understands DM 3.x annotations) that relies on optional temporal
service. Otherwise I'd have to manually track/wait Foobar service after
asynchronous CM update (with a custom default impl)
{code}
public class FoobarIT
{
@Rule
public TestRule testRule = new DependencyManagerTestRule() { protected
Object getWrapped() { return FoobarIT.this; }};
@Inject
private BundleContext bundleContext;
@ServiceDependency(name = Foobar.PID, timeout = 6)
private Foobar foobar;
@ServiceDependency
private ConfigurationAdmin configurationAdmin;
@ResourceDependency(filter = ( + ResourceHandler.PATH + =/META-INF/
+ Foobar.PID + .cfg))
private URL url;
@Init
MapString, ? init() throws IOException
{
Bundle bundle = this.bundleContext.getBundle();
String location = bundle.getLocation();
Configuration configuration =
this.configurationAdmin.getConfiguration(Foobar.PID, location);
Properties properties = new Properties();
try (InputStream inputStream = this.url.openStream();)
{
properties.load(inputStream);
}
configuration.update((Dictionary) properties);
return Collections.singletonMap(Foobar.PID + .required,
Boolean.FALSE);
}
@Test
public void test() {
this.foobar.blockUntilConfigured();
}
}
{code}
The verification that JUnit test case is ready for evaluation
(required-available) is done inside evaluate method. I got it working by
patching {{TemporalServiceDependencyImpl}} just by removing the setRequired()
method that would have been executed-failed when {{init()}} method of the test
returned.
ComponentProvider is still proprietary piece of code that functions in place of
runtime bundle (reading annotations directly from the classes themselves), but
I guess it's clear what it does.
{code}
public abstract class DependencyManagerTestRule implements TestRule
{
private DependencyManager dependencyManager;
public DependencyManagerTestRule()
{
Object wrapped = this.getWrapped();
Bundle bundle = FrameworkUtil.getBundle(wrapped.getClass());
BundleContext bundleContext = bundle.getBundleContext();
this.dependencyManager = new DependencyManager(bundleContext);
}
protected abstract Object getWrapped();
@Override
public Statement apply(final Statement wrapped, Description description)
{
Class? extends Object testClass = description.getTestClass();
ComponentProvider componentProvider = new ComponentProvider(
this.dependencyManager, testClass);
final Component component = componentProvider.get();
Object implementation = this.getWrapped();
component.setImplementation(implementation);
Statement statement = new Statement()
{
@Override
public void evaluate() throws Throwable
{
DependencyManagerTestRule.this.dependencyManager.add(component);
@SuppressWarnings(unchecked)
ListDependency dependencies = component.getDependencies();
for (Dependency dependency : dependencies)
{
boolean required = dependency.isRequired();
boolean available = dependency.isAvailable();
if (!available required)
{
ComponentDependencyDeclaration
componentDependencyDeclaration = (ComponentDependencyDeclaration) dependency;
String type = componentDependencyDeclaration.getType();
[jira] [Commented] (FELIX-4847) Make TemporalServiceDependency always available (and/or allow it to be optional)
[
https://issues.apache.org/jira/browse/FELIX-4847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14486830#comment-14486830
]
Pierre De Rop commented on FELIX-4847:
--
Hello Tuomas,
I'm not sure I understand the use case, can you provide a sort of pseudo code
which describes the usecase, or even a bndtools unit test ?
in the latest release, we removed the interface but you can still create a
timed (and required) service dependency using the following method in the
DependencyManager class, or the DependencyActivatorBase class:
{code}
public ServiceDependency createTemporalServiceDependency(long timeout);
{code}
So, as you said in your 1) point, the returned dependency will (unless there is
a bug) always remain available even if the target dependency becomes
unavailable.
Regarding the second point, I'm not sure I understand why we should support
optionality for temporal dependencies ? Indeed, if a dependency is optional,
you will be unbound from it when the dependency goes away (not a problem since
it's an optional dependency) or you can use a null object pattern and when the
dependency is not available, then a null object will replace the actual missing
dependency.
but may be there is a bug somwhere ? so that would be great if you could
provide a test case for all this.
thank you.
Make TemporalServiceDependency always available (and/or allow it to be
optional)
Key: FELIX-4847
URL: https://issues.apache.org/jira/browse/FELIX-4847
Project: Felix
Issue Type: Wish
Components: Dependency Manager
Affects Versions: dependencymanager-3.2.0
Reporter: Tuomas Kiviaho
I wanted to use temporal service to wait for CM update thread to finish what
it's doing (because the spec doesn't have a non-parallel version).
Everything worked fine until JUnit test rule said that the component isn't
ready yet. I was merely checking that every required dependency was also
available and to my surprise the temporal service was marked unavailable
until the CM had completed what it was doing.
1) Shouldn't temporal service be always available externally via available
property and keep track on the actual state only internally? This approach
might not be backwards compatible.
2) Could temporal service be allowed to be marked as optional. This would
suit my use case, but it feels like a 'golden hammer' approach because it
alters component's state machine behavior a bit which in turn can be harmful
for other use cases.
As a workaround I'd have to differentiate the dependencies somehow from each
other, but I see that the 4.x has removed the dedicated interface that I was
thinking of relying upon to.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FELIX-4548) Implement the missing errors registration
[ https://issues.apache.org/jira/browse/FELIX-4548?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14487325#comment-14487325 ] Carsten Ziegeler commented on FELIX-4548: - Thanks [~tkb] . I've applied your patch. I think I've found a solution for the traversal, instead of using the exception type we can use the exception object itself. I've adjusted the patch Implement the missing errors registration - Key: FELIX-4548 URL: https://issues.apache.org/jira/browse/FELIX-4548 Project: Felix Issue Type: Sub-task Components: HTTP Service Affects Versions: http-2.4.0 Reporter: Simone Tripodi Assignee: Carsten Ziegeler Fix For: http.base-3.0.0 Attachments: FELIX-4548.patch, felix-4548-exception-hierarchy.patch Current implementation of RFC189 does not support yet errors registration. Questions are: * do we want to have a servlet (or maybe a filter fits better) implementation which serves a static file whenever an HTTP error is detected? or * looking for a solution that delegates the underlying servlet container? -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (FELIX-4848) Split ResolverImpl
[ https://issues.apache.org/jira/browse/FELIX-4848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14487790#comment-14487790 ] Richard S. Hall commented on FELIX-4848: I am always humored by such rules (e.g., a file shouldn't be over X many lines, etc.)...as always, it just depends. I don't see how distributing complex code over multiple files makes it any more tractable for someone than having those same details in a single file. Just an observation. Split ResolverImpl -- Key: FELIX-4848 URL: https://issues.apache.org/jira/browse/FELIX-4848 Project: Felix Issue Type: Improvement Components: Resolver Affects Versions: resolver-1.0.0 Reporter: Christian Schneider Fix For: resolver-1.2.0 ResolverImpl currently contains about 2300 lines of code. That is way too big for a single class. I looked into it and found that the checkDynamicPackageSpaceConsistency and checkPackageSpaceConsistency methods and their dependent methods form a nice subset. I would move that into a class ConsistencyCheck. Both would share all of the inner classes of ResolverImpl but nothing else. So I think i would make sense to move these inner classes to separate files. These changes should nicely split the classes into ResolverImpl : 1400 lines ConsistencyCheck : 600 lines -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (FELIX-4848) Split ResolverImpl
Christian Schneider created FELIX-4848: -- Summary: Split ResolverImpl Key: FELIX-4848 URL: https://issues.apache.org/jira/browse/FELIX-4848 Project: Felix Issue Type: Improvement Components: Resolver Affects Versions: resolver-1.0.0 Reporter: Christian Schneider Fix For: resolver-1.2.0 ResolverImpl currently contains about 2300 lines of code. That is way too big for a single class. I looked into it and found that the checkDynamicPackageSpaceConsistency and checkPackageSpaceConsistency methods and their dependent methods form a nice subset. I would move that into a class ConsistencyCheck. Both would share all of the inner classes of ResolverImpl but nothing else. So I think i would make sense to move these inner classes to separate files. These changes should nicely split the classes into ResolverImpl : 1400 lines ConsistencyCheck : 600 lines -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (FELIX-4847) Allow TemporalServiceDependency to be optional
[ https://issues.apache.org/jira/browse/FELIX-4847?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Tuomas Kiviaho updated FELIX-4847: -- Summary: Allow TemporalServiceDependency to be optional (was: Make TemporalServiceDependency always available (and/or allow it to be optional)) Allow TemporalServiceDependency to be optional -- Key: FELIX-4847 URL: https://issues.apache.org/jira/browse/FELIX-4847 Project: Felix Issue Type: Wish Components: Dependency Manager Affects Versions: dependencymanager-3.2.0 Reporter: Tuomas Kiviaho I wanted to use temporal service to wait for CM update thread to finish what it's doing (because the spec doesn't have a non-parallel version). Everything worked fine until JUnit test rule said that the component isn't ready yet. I was merely checking that every required dependency was also available and to my surprise the temporal service was marked unavailable until the CM had completed what it was doing. 1) Shouldn't temporal service be always available externally via available property and keep track on the actual state only internally? This approach might not be backwards compatible. 2) Could temporal service be allowed to be marked as optional. This would suit my use case, but it feels like a 'golden hammer' approach because it alters component's state machine behavior a bit which in turn can be harmful for other use cases. As a workaround I'd have to differentiate the dependencies somehow from each other, but I see that the 4.x has removed the dedicated interface that I was thinking of relying upon to. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (FELIX-4541) Add support for multiple patterns in servlet/filter registration
[ https://issues.apache.org/jira/browse/FELIX-4541?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14486936#comment-14486936 ] Carsten Ziegeler commented on FELIX-4541: - Thanks again [~tkb] for your patch, it's applied in rev 1672256 Add support for multiple patterns in servlet/filter registration Key: FELIX-4541 URL: https://issues.apache.org/jira/browse/FELIX-4541 Project: Felix Issue Type: Sub-task Components: HTTP Service Reporter: J.W. Janssen Fix For: http.base-3.0.0 Attachments: FELIX-4541.patch, FELIX-4541.trunk-1671836.patch RFC-189 allows servlets and filters to be registered with multiple paths (and even regex's for filters). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (FELIX-4548) Implement the missing errors registration
[ https://issues.apache.org/jira/browse/FELIX-4548?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Thomas Baier updated FELIX-4548: Attachment: felix-4548-exception-hierarchy.patch There are two things still missing in the current implementation: - handling of error pages registering with 4xx or 5xx - for exceptions the exception hierarchy should be searched for a matching error page felix-4548-exception-hierarchy.patch tries to fix the second part. There is one problem I ran into: in order to search the exception hierarchy I use Class.forName to get the exception class. But this works only for exceptions that are on the class path for the bundle containing the whiteboard implementation (?). For a custom exception I created initially in the integration test I got a ClassNotFound exception. Implement the missing errors registration - Key: FELIX-4548 URL: https://issues.apache.org/jira/browse/FELIX-4548 Project: Felix Issue Type: Sub-task Components: HTTP Service Affects Versions: http-2.4.0 Reporter: Simone Tripodi Assignee: Carsten Ziegeler Fix For: http.base-3.0.0 Attachments: FELIX-4548.patch, felix-4548-exception-hierarchy.patch Current implementation of RFC189 does not support yet errors registration. Questions are: * do we want to have a servlet (or maybe a filter fits better) implementation which serves a static file whenever an HTTP error is detected? or * looking for a solution that delegates the underlying servlet container? -- This message was sent by Atlassian JIRA (v6.3.4#6332)
