[mapserver-dev] Using updateFromString on existing items

[Tamas Szekeres]

Getting back to this problem, I might think this is a bug in loadLayer,
where the projection object should be reset before loading the new one.
updateFromString should always work on already initialized parameters
either of string type or the more complex types. In this regard
loadProjection (and loadExpression) should work like getString to clean up
the existing object to avoid memory leaks.

So what about changing loadProjection this way?

@@ -1355,9 +1355,8 @@ static int loadProjection(projectionObj *p)
 #ifdef USE_PROJ

   if ( p->proj != NULL ) {
-    msSetError(MS_MISCERR, "Projection is already initialized. Multiple
projection definitions are not allowed in this object. (line %d)",
-               "loadProjection()", msyylineno);
-    return(-1);
+    msFreeProjection(p);
+    msInitProjection(p);
   }

Thanks,

Tamas


2017-10-10 21:26 GMT+02:00 Lime, Steve D (MNIT) <steve.lime at state.mn.us>:

> Not sure, seems easy enough to test. I’ll try and take a look at the Swig
> files and report back. If you’re going to reset and replace all existing
> classes, styles, labels, etc… you’d want the reference counts to be
> affected wouldn’t you?
>
>
>
> *From:* Tamas Szekeres [mailto:szekerest at gmail.com]
> *Sent:* Tuesday, October 10, 2017 1:48 PM
>
> *To:* Lime, Steve D (MNIT) <steve.lime at state.mn.us>
> *Cc:* mapserver-dev at lists.osgeo.org
> *Subject:* Re: [mapserver-dev] Using updateFromString on existing items
>
>
>
> Adding a separate function is fine for me. Would that be easy to
> implement? I've been thinking that freeLayer would do the trick, but that
> is bound to a reference counting approach, which prevents from calling this
> method in my use case.
>
>
>
> Tamas
>
>
>
>
>
> 2017-10-10 20:36 GMT+02:00 Lime, Steve D (MNIT) <steve.lime at state.mn.us>:
>
> I was thinking more about this and I think there are several possibilities
> if you want to completely repopulate a layer (or other object) from a
> string. Personally, I’d leave the updateFromString() method alone and add a
> reset/init or whatever method that basically free’s the object and then
> re-initializes it resulting in an object that is ready to load a definition
> into. For a layer that would clear out projections, classes, etc…, for a
> class that would clear out styles, and so on… Code would look something
> like:
>
>
>
>   $layer = $map->getLayerByName(‘foo’);
>
>   $layer->reset();
>
>   $layer->updateFromString($fullLayerDefinitionString);
>
>
>
> One could also add new method to explicitly do the reset and update (e.g.
> resetFromString) or perhaps add a flag to the updateFromString method to
> reset.
>
>
>
> Steve
>
>
>
> *From:* mapserver-dev [mailto:mapserver-dev-bounces at lists.osgeo.org] *On
> Behalf Of *Lime, Steve D (MNIT)
> *Sent:* Monday, October 09, 2017 11:12 AM
> *To:* Tamas Szekeres <szekerest at gmail.com>
>
>
> *Cc:* mapserver-dev at lists.osgeo.org
> *Subject:* Re: [mapserver-dev] Using updateFromString on existing items
>
>
>
> I think we need to spend a little time looking at the code and the
> original RFC to properly document the “as is” behavior and then determine
> if any changes are necessary. Looking at the layer code I think my
> reference to “minor edits” is wrong. If you update a layer from a string in
> MapScript and that string contains classes then **new** classes are added
> to the layer object. If you want to update an existing class you’d have to
> update it directly from a string, not via the parent layer. Resetting the
> object would be a pretty drastic change. It may be that only projections
> need some sort of intervention to clear things out…
>
>
>
> Steve
>
>
>
> *From:* Tamas Szekeres [mailto:szekerest at gmail.com <szekerest at gmail.com>]
> *Sent:* Monday, October 09, 2017 10:18 AM
> *To:* Lime, Steve D (MNIT) <steve.lime at state.mn.us>
> *Cc:* mapserver-dev at lists.osgeo.org
> *Subject:* Re: [mapserver-dev] Using updateFromString on existing items
>
>
>
> Hi Steve,
>
>
>
> It is hard to interpret what are the allowed "minor edits" in such case.
> Neither the documentation or the corresponting rfc provides any guidance.
>
> One can think a reasonable use case to completely deserialize the object
> state by using these methods.
>
>
>
> Do we have a simple approach to reset the state of the object (as it was
> newly created) before calling the updateFromString method?
>
>
>
> Thanks,
>
>
>
> Tamas
>
>
>
>
>
>
>
> 2017-10-09 16:23 GMT+02:00 Lime, Steve D (MNIT) <steve.lime at state.mn.us>:
>
> The original use case was to facilitate minor edits to existing object
> properties but not sub-objects. For example, you can't replace all the
> classes in a layer through a string update to the layer itself. We
> certainly could make all that work cleaner if there were demand.
>
> At least you get a proper error message... ;-)
>
>
>
> Steve
> ------------------------------
>
> *From:* mapserver-dev <mapserver-dev-bounces at lists.osgeo.org> on behalf
> of Tamas Szekeres <szekerest at gmail.com>
> *Sent:* Sunday, October 8, 2017 12:49:55 PM
> *To:* mapserver-dev at lists.osgeo.org
> *Subject:* [mapserver-dev] Using updateFromString on existing items
>
>
>
> Hi Devs,
>
>
>
> Is that a supported use case to apply updateFromString on an existing item
> or it should only be used only on a newly created item?
>
>
>
> For example when I use layerObj.updateFromString on an existing layer I
> get the following error
>
>
>
> loadProjection(): General error message. Projection is already
> initialized. Multiple projection definitions are not allowed in this
> object. (line 10)
>
>
>
> This is because we don't have a proper cleanup code on the existing layer
> before calling loadLayer on it. The same applies to the other object types.
>
>
>
> Thanks,
>
> Tamas
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/mapserver-dev/attachments/20171112/f5867cf3/attachment.html>