fcrepo4-labs / fcrepo-api-x

Fedora API Extension Framework
Apache License 2.0
10 stars 11 forks source link

ExposedServiceUrlAnalyzer update() evaluation for Conflicting Extensions does not work as intended #150

Open christopher-johnson opened 7 years ago

christopher-johnson commented 7 years ago

The code here tries to read e1 and e2 as uris when these are strings. The method also does not throw if it gets to the warning even though the test is expecting one. Instead it ends up comparing two nulls and returning an NPE.

I assume that ServiceExposingBinding match(..) will always need an update first, which does not happen with init.await(). When I replacedinit.await() with this.update() and removed toTest.update() from the test setup, the only test that failed was conflictingExtensionsTest because the update() returns an NPE. If e1.uriand e2.uri are replaced with e1.toString() ande2.toString(), then the test still fails because of the fail "Should have thrown an exception upon update" expectation, that does not exist in the method.

I am not sure exactly why you introduce the Initializer interface for Runnable tasks, but this makes the whole thing rather difficult. How does this init method work? I do not get why you need to store the initializer interface in a private variable called init at all. Wouldn't it be clearer to use Runnable methods on instantiated tasks if you need to?