Confusing exception with incrementalBuildEnabled

[muffl0n]

For better comprehensibility I pushed my test-project to muffl0n/wro4j-scss-test.

When I execute maven with goal compile on this project (using current 1.7.7-SNAPSHOT) I encounter this exception when compiling after changed the file /src/main/webapp/scss/header.scss:

[ERROR] Cannot apply a function on @import resource: /scss/header.scss. Ignoring it.
ro.isdc.wro.WroRuntimeException: Change detected. No need to continue processing
    at ro.isdc.wro.maven.plugin.support.ResourceChangeHandler$1.apply(ResourceChangeHandler.java:103)
    at ro.isdc.wro.maven.plugin.support.ResourceChangeHandler$1.apply(ResourceChangeHandler.java:1)
    at ro.isdc.wro.maven.plugin.support.ResourceChangeHandler$3.onImportDetected(ResourceChangeHandler.java:167)
    at ro.isdc.wro.model.resource.processor.impl.css.AbstractCssImportPreProcessor.findImportedResources(AbstractCssImportPreProcessor.java:174)
    at ro.isdc.wro.model.resource.processor.impl.css.AbstractCssImportPreProcessor.parseCss(AbstractCssImportPreProcessor.java:129)
    at ro.isdc.wro.model.resource.processor.impl.css.AbstractCssImportPreProcessor.process(AbstractCssImportPreProcessor.java:98)
    at ro.isdc.wro.model.resource.processor.decorator.ProcessorDecorator.process(ProcessorDecorator.java:89)
    at ro.isdc.wro.model.resource.processor.decorator.ExceptionHandlingProcessorDecorator.process(ExceptionHandlingProcessorDecorator.java:56)
    at ro.isdc.wro.maven.plugin.support.ResourceChangeHandler.forEachCssImportApply(ResourceChangeHandler.java:158)
    at ro.isdc.wro.maven.plugin.support.ResourceChangeHandler.detectChangeForCssImports(ResourceChangeHandler.java:95)
    at ro.isdc.wro.maven.plugin.support.ResourceChangeHandler.isResourceChanged(ResourceChangeHandler.java:83)
    at ro.isdc.wro.maven.plugin.AbstractWro4jMojo.getIncrementalGroupNames(AbstractWro4jMojo.java:331)
    at ro.isdc.wro.maven.plugin.AbstractWro4jMojo.getTargetGroupsAsList(AbstractWro4jMojo.java:290)
    at ro.isdc.wro.maven.plugin.Wro4jMojo.doExecute(Wro4jMojo.java:139)
    at ro.isdc.wro.maven.plugin.AbstractWro4jMojo.execute(AbstractWro4jMojo.java:164)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:101)
    [...]
    at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:352)

Everything is generated correctly but this error does confuse me and will confuse others.

The problem seems to be the WroRuntimeException that is thrown in ro.isdc.wro.maven.plugin.support.ResourceChangeHandler.detectChangeForCssImports():

        if (isImportChanged) {
          changeDetected.set(true);
          // no need to continue
          throw new WroRuntimeException("Change detected. No need to continue processing");
        }

and logged in ro.isdc.wro.maven.plugin.support.ResourceChangeHandler.createCssImportProcessor():

        try {
          func.apply(importedUri);
          remember(Resource.create(importedUri, ResourceType.CSS));
        } catch (final Exception e) {
          getLog().error("Cannot apply a function on @import resource: " + importedUri + ". Ignoring it.", e);
        }

I'm not quite sure how to handle this. I feel like throwing an exception if the file has changed is not the right way. But I'm not sure what option would be better.

[alexo]

The exception is thrown to short circuit the change detection logic. Once a group has been detected as changed, there is no need to continue checking other resources.

I understand your confusion. I think one approach would be throwing a dedicated exception instead, and treating it differently (not logging). Other thoughts?

[muffl0n]

First thing I learned when programming Java was: "Don't use exceptions if it's not really an exception." :) In other words: I would not throw an exception for a logic that is totally ok and normal. Couldn't apply() return something other than Void? Maybe a bool that tells the calling Processor to continue or not?

I'm sorry that I can not be more helpful but I'm still trying to understand the Processor/func-logic in ResourceChangeHandler and haven't got so far as I wished I had. :)

But maybe throwing a special exception that is treated differently would be a solution for the time being at least.

[alexo]

I agree that in this case the exception is misused. Changing the func to return a boolean could work, but it require few changes. If you have time, could you give it a try and see how it looks like? The unit tests should help to identify if everything works as expected and nothing is broken.

[muffl0n]

I gave my best in PR #201. I'm really not sure that this does not break anything. My project builds just fine and the tests don't fail. But I have kind of a mixed feeling about this whole thing. :)

Looking forward to hearing your opinion on this.

[alexo]

Looks good to me. Will merege it soon...

[muffl0n]

Thank you! Looks good for my coworkers and me so far. :)

[alexo]

@muffl0n I should thank you :).. Most of the recent issues are fixed by you. Great job! If you have any other suggestions or ideas, please let me know.

Btw, would you like me to add you to the wro4j github organization? I really appreciate your help.

[muffl0n]

Just polishing the edges of a really nice plugin. Nice to see that you are keeping on maintaining the code and accept PRs. You can add me to the organization. I don't know exactly what this means, but it sound's cool. :)

[alexo]

I've created the github organization to host other wro4j related project. The wro4j code base is not yet migrated there, but I'm planning to do it at some point of the future. Also the organization is a good place for documentation and other small projects which have similar concerns.