There's lots of insanity going on with things like WikiEngine.getAllInlinedImagePatterns, which do nothing except delegate to JSPWikiMarkupParser.getImagePatterns(WikiEngine)... which just rips through the jspwiki.properties and returns a String collection. It's called just once, by... wait for it... WikiEngine.
Originally, it was only used by the MarkupParser, so it made sense to keep it in there, as it would localize any configuration into MarkupParser itself. I don't know why and how WikiEngine suddenly became the owner of this method. Does it need to know the inlined image patterns? Shouldn't this be something which is completely owned by MarkupParser?
Now, I know why this was done. It was to prevent WikEngine from being gigantic. That makes sense, but it seems to me we could be doing this a bit differently.
I think it's just an honest refactoring mistake :-). I personally think it's better that objects themselves own their configuration. I say let's get rid of WikiEngine.getAllInlinedImagePatterns(), if we can, and other methods in WikiEngine which hold config data in WikiEngine which acts only as a facade.
For example, we could create a new class called WikiConfiguration (or something like it) that configures and stores state for things like that, like inline images, the names of configured spaces, etc. This would be configured just once, at startup. All of the gnarly property-parsing stuff could go there. One variant on it could be PropertyFileConfiguration (which reads property files). Another could be a MemoryConfiguration for testing purposes. Embedders could write their own if they wanted to embed a wiki that "just works" without much configuration at all, or one that reads from their own config files
I'm kind of two minds about this. First, a Property list is essentially a key-value store, which is what we would essentially duplicate. Second, it would potentially introduce a fairly massive incompatibility between 2.x and 3.0. Third, it violates the information hiding principle by forcing a configuration object to know stuff about the internal workings of another object. So it could really only be a key-value store, which Properties already is. So I would go for reuse of existing stuff here.
However, we *could* extend java.utils.Properties to provide anything that we need. For example, allow the JSPWikiProperties object to be a MXBean. Which in turn would probably mean that we should have a "CONFIG_CHANGED" event, which would allow certain optimizations. Reusing java.util.Properties also means that we can reuse a lot of the stuff like CommentedProperties. And we could also provide an immutable version of the Properties object for subclasses via WikiEngine.getProperties() or something.
But, and this is a big one, I don't believe that we should do this for 3.0. We *can* refactor WikiEngine so that it has a better interface for getting the configuration, which is probably a good idea.
/Janne
