Using updateFromString on existing items

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Using updateFromString on existing items

Tamas Szekeres
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

_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: Using updateFromString on existing items

Lime, Steve D (MNIT)

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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

_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: Using updateFromString on existing items

Tamas Szekeres
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) <[hidden email]>:

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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


_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: Using updateFromString on existing items

Lime, Steve D (MNIT)

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:[hidden email]]
Sent: Monday, October 09, 2017 10:18 AM
To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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

 


_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: Using updateFromString on existing items

Lime, Steve D (MNIT)

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:[hidden email]] On Behalf Of Lime, Steve D (MNIT)
Sent: Monday, October 09, 2017 11:12 AM
To: Tamas Szekeres <[hidden email]>
Cc: [hidden email]
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 [[hidden email]]
Sent: Monday, October 09, 2017 10:18 AM
To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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

 


_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: Using updateFromString on existing items

Tamas Szekeres
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) <[hidden email]>:

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:[hidden email]] On Behalf Of Lime, Steve D (MNIT)
Sent: Monday, October 09, 2017 11:12 AM
To: Tamas Szekeres <[hidden email]>


Cc: [hidden email]
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 [[hidden email]]
Sent: Monday, October 09, 2017 10:18 AM
To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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

 



_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: Using updateFromString on existing items

Lime, Steve D (MNIT)

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:[hidden email]]
Sent: Tuesday, October 10, 2017 1:48 PM
To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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:[hidden email]] On Behalf Of Lime, Steve D (MNIT)
Sent: Monday, October 09, 2017 11:12 AM
To: Tamas Szekeres <[hidden email]>


Cc: [hidden email]
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 [[hidden email]]
Sent: Monday, October 09, 2017 10:18 AM
To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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

 

 


_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev
Reply | Threaded
Open this post in threaded view
|

Re: 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) <[hidden email]>:

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:[hidden email]]
Sent: Tuesday, October 10, 2017 1:48 PM


To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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:[hidden email]] On Behalf Of Lime, Steve D (MNIT)
Sent: Monday, October 09, 2017 11:12 AM
To: Tamas Szekeres <[hidden email]>


Cc: [hidden email]
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 [[hidden email]]
Sent: Monday, October 09, 2017 10:18 AM
To: Lime, Steve D (MNIT) <[hidden email]>
Cc: [hidden email]
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) <[hidden email]>:

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 <[hidden email]> on behalf of Tamas Szekeres <[hidden email]>
Sent: Sunday, October 8, 2017 12:49:55 PM
To: [hidden email]
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

 

 



_______________________________________________
mapserver-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/mapserver-dev