GeoTools

Process parameters not passed to RenderingTransformation invert methods

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 8.0-M4
  • Fix Version/s: None
  • Component/s: process
  • Labels:
    None

Description

The RenderingProcess invertQuery and invertGridGeometry methods provide the process parameters as arguments. This is necessary to allow the process to modify the query driven by the provided parameters. Currently the parameters are not being passed correctly. This is due to bugs in both RenderingProcessFunction and AnnotationDrivenProcessFactor.InvokeMethodRenderingProcess.

The fix for RenderingProcessFunction is to expose the parameter-marshalling logic from ProcessFunction as a callable method,
and call it when to create the required input argument.

The fix for InvokeMethodRenderingProcess is to correctly pass along the input parameters received in the invertQuery/invertGridGeometry calls.

  1. File
    process.diff
    08/Mar/12 6:09 PM
    12 kB
    Martin Davis
  2. File
    testProcess.diff
    04/Apr/12 11:28 AM
    6 kB
    Martin Davis
  3. File
    testProcess.diff
    03/Apr/12 2:48 PM
    4 kB
    Martin Davis

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Martin Davis added a comment -
Update: the AnnotationDrivenProcessFactor.InvokeMethodRenderingProcess is actually correct. It just wasn't obvious that the invert method parameters could also be annotation-driven. I'll try and add a comment to make this clearer.
Show
Martin Davis added a comment - Update: the AnnotationDrivenProcessFactor.InvokeMethodRenderingProcess is actually correct. It just wasn't obvious that the invert method parameters could also be annotation-driven. I'll try and add a comment to make this clearer.
Hide
Permalink
Martin Davis added a comment -
Patch attached
Show
Martin Davis added a comment - Patch attached
Hide
Permalink
Andrea Aime added a comment -
Tsk tsk, no test, no party
Show
Andrea Aime added a comment - Tsk tsk, no test, no party
Hide
Permalink
Andrea Aime added a comment -
Btw, the patch is otherwise (very) good
Show
Andrea Aime added a comment - Btw, the patch is otherwise (very) good
Hide
Permalink
Martin Davis added a comment -
Andrea, does this mean you are not accepting this patch without a unit test?

I agree that a test would be nice. Is there an existing test that could be modified slightly to test this bug?
Show
Martin Davis added a comment - Andrea, does this mean you are not accepting this patch without a unit test? I agree that a test would be nice. Is there an existing test that could be modified slightly to test this bug?
Hide
Permalink
Martin Davis added a comment -
I would like to start working on a test for this patch, to allow it to be accepted.

I'm thinking of creating a simple annotation-driven rendering transformation, and inside it validating that invertQuery and invertGridGeometry are called appropriately. I suppose I'll have to invoke this through the StreamingRenderer, or is there a more direct way to invoke it (eg by calling the function directly?)

Any suggestions on how best to create such a test, and code to copy if possible?
Show
Martin Davis added a comment - I would like to start working on a test for this patch, to allow it to be accepted. I'm thinking of creating a simple annotation-driven rendering transformation, and inside it validating that invertQuery and invertGridGeometry are called appropriately. I suppose I'll have to invoke this through the StreamingRenderer, or is there a more direct way to invoke it (eg by calling the function directly?) Any suggestions on how best to create such a test, and code to copy if possible?
Hide
Permalink
Andrea Aime added a comment -
No need to call into the streaming renderer, if the fix is in the process module you just need to create a fake annotated process that stores the params received in a field,
call it as a rendering transformation and make sure the params are getting passed into it
Show
Andrea Aime added a comment - No need to call into the streaming renderer, if the fix is in the process module you just need to create a fake annotated process that stores the params received in a field, call it as a rendering transformation and make sure the params are getting passed into it
Hide
Permalink
Martin Davis added a comment -
Attached is a patch with a unit test for the passing of parameters to the invertQuery method
Show
Martin Davis added a comment - Attached is a patch with a unit test for the passing of parameters to the invertQuery method
Hide
Permalink
Andrea Aime added a comment -
Martin, did you forget to include the VectorIdentityRTProcess java file?
Anyways, I guess you are returning the data that is passed throught the invert query method?
If so it looks good, do you have commit access? If not, and want it, ask on the ml
Otherwise I can commit the patch myself
Show
Andrea Aime added a comment - Martin, did you forget to include the VectorIdentityRTProcess java file? Anyways, I guess you are returning the data that is passed throught the invert query method? If so it looks good, do you have commit access? If not, and want it, ask on the ml Otherwise I can commit the patch myself
Hide
Permalink
Martin Davis added a comment -
Argh, yes, the diff was missing a file. I'll upload a replacement.

I'm actually checking that the value passed to the invertQuery is the same as the one passed to the execute method. This should test that all parameters are being passed correctly. (There's lots of ways this could be checked - feel free to suggest some better way).

I don't have commit access right now, so if you can patch this that would be great. And I'll ask about getting access.
Show
Martin Davis added a comment - Argh, yes, the diff was missing a file. I'll upload a replacement. I'm actually checking that the value passed to the invertQuery is the same as the one passed to the execute method. This should test that all parameters are being passed correctly. (There's lots of ways this could be checked - feel free to suggest some better way). I don't have commit access right now, so if you can patch this that would be great. And I'll ask about getting access.
Hide
Permalink
Martin Davis added a comment -
This is a replacement for the earlier testProcess.diff file, containing the VectorIdentityRTProcess.java file
Show
Martin Davis added a comment - This is a replacement for the earlier testProcess.diff file, containing the VectorIdentityRTProcess.java file
Hide
Permalink
Andrea Aime added a comment -
Patch(es) committed on trunk
Show
Andrea Aime added a comment - Patch(es) committed on trunk

People

  • Assignee:
    Andrea Aime
    Reporter:
    Martin Davis
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: