Re: [udig-devel] Problem with OpAction property value checks

[andrea antonello <andrea.antonello@xxxxxxxxx>]

Hi Frank,

> Andrea, on the local repository I already tried it and had no trouble with
> it. I There is only a switch from internal URL to String key of a
> java.util.Map for the inner cache storage.  guess the changes are minor and
> the Interface doesn't changed. I applied this change since I created the
> issue and we had no problems with our application.
> We could start creating a test case for it but right now I would say:Its
> always possible to convert an URL to String and I wouldn't expect any side
> effects right here.
> Would you agree after reviewing the patch?

Yeah, I definitely agree, the equals on URL is always a mess. It was
more on the OpAction I had troubles to get what's going on.

I applied your patches on the main repo. If someone wants to
reviewfurther, here is the visual diff ready to be commented:
http://tinyurl.com/233gcmr

Ciao,
Andrea



> Frank
>
>
>
> 2010/11/8 andrea antonello <andrea.antonello@xxxxxxxxx>
>>
>> Hi Frank,
>> I can look into it in a couple of hours and apply the patches. In case
>> are you around to test the outcome on a uDig checkout?
>>
>> Ciao
>> Andrea
>>
>>
>>
>>
>> On Mon, Nov 8, 2010 at 9:28 AM, Frank Gasdorf
>> <fgdrf@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> > Hello Udig'ers,
>> >
>> > I'd like to ask, whether someone could commit the patch for, in my
>> > opinion
>> > major performance bug. Having an url in a java.util.Map as a key would
>> > led
>> > into trouble having url's that can't be resolved. Please have a look at
>> > the
>> > issues listed below. If you have any questions, feel free to ask.
>> >
>> > Thanks a lot
>> > Frank
>> >
>> > 2010/6/2 Frank Gasdorf <fgdrf@xxxxxxxxxxxxxxxxxxxxx>
>> >>
>> >> Hello again,
>> >> I created two issues for operation improvements:
>> >>  UDIG-1594 : GeometryProperty has timeout on Layer based on "virtual"
>> >> services
>> >>  UDIG-1586 : When does a operation re-check the properties
>> >> Could somebody look at and comment these? For each I attached a patch,
>> >> maybe it is a point to start from ...
>> >> Cheers
>> >> Frank
>> >> 2009/10/30 Frank Gasdorf <fgdrf@xxxxxxxxxxxxxxxxxxxxx>
>> >>>
>> >>> 2009/10/30 Jody Garnett <jody.garnett@xxxxxxxxx>
>> >>>>
>> >>>> > - i created an operation, used the
>> >>>> > net.refractions.udig.ui.operation
>> >>>> > extension point to get an Action for the targetClass
>> >>>> > net.refractions.udig.project.ILayer
>> >>>>
>> >>>> Okay- so your operation should work on a Layer; or anything that can
>> >>>> adapt to a Layer - so probably a Map as well (assuming the current
>> >>>> layer).
>> >>>
>> >>> Right.
>> >>>>
>> >>>> > - subclassed AbstractPropertyValue and created my own PropertyValue
>> >>>> > class
>> >>>> > (checks preferences for layer using a preferences store)
>> >>>>
>> >>>> > - the user select a layer, changed the preference for the selected
>> >>>> > layer and
>> >>>> > as a result an operation should be available within the context
>> >>>> > menu
>> >>>> > of the
>> >>>> > still selected layer
>> >>>> > BUT the action (OpAction) is not visible (PropertyValue would
>> >>>> > return
>> >>>> > true)
>> >>>> > because the selection didn't changed.
>> >>>> > If the customer deselects the layer and selects it again it works.
>> >>>>
>> >>>> Oh I see; so you somehow want to ask it to check again. I am pretty
>> >>>> sure it only wants to check once for speed (rather then making the
>> >>>> right click operation slow). I wonder if the property value could
>> >>>> listen to the Layer? Or would that also be slow.
>> >>>>
>> >>> I guess the property value could listen to the layer but the changes
>> >>> occurs not on the layer or layer related objects. It's in the
>> >>> PreferencesStore, updated from a StyleEditorPage. Or is it even
>> >>> possible to
>> >>> set some properties for a layer? So maybe the PropertyValue
>> >>> implementation
>> >>> could listen to the layer(-properties).
>> >>>
>> >>>>
>> >>>> > What I'm doing wrong? In my opinion the OpAction.updateEnablement
>> >>>> > method
>> >>>> > should not check whether the internal selection changed (see
>> >>>> > attached
>> >>>> > file
>> >>>> > OpAction.patch). But it is possible to run into performance issues
>> >>>> > if
>> >>>> > PropertyValue implementations are very heavy.
>> >>>>
>> >>>> Agreed.
>> >>>>
>> >>>> > Could somebody comment my thoughts? If required I can create an
>> >>>> > issue
>> >>>> > for this.
>> >>>>
>> >>>> I would like to create an issue for this so that:
>> >>>> a) we can talk to Jesse about it (who write the PropertyValue code)
>> >>>> b) work on a patch for his review
>> >>>>
>> >>>> Jody
>> >>>
>> >>> OK, i create an issue and upload a patch. Jody, have you looked into
>> >>> the
>> >>> appended? Do you thing it solves the problem? I've seen there are some
>> >>> implementations but not to many and all of them are not so expensive
>> >>> at
>> >>> runtime, are they? Thanks for comments so far.
>> >>>
>> >>> Frank
>> >>
>> >
>> >
>> > _______________________________________________
>> > User-friendly Desktop Internet GIS (uDig)
>> > http://udig.refractions.net
>> > http://lists.refractions.net/mailman/listinfo/udig-devel
>> >
>> >
>
>