Opened 17 years ago
Last modified 15 years ago
#2197 assigned defect
Cleanup code that strips namespaces in msWFSDescribeFeatureType()
Reported by: | dmorissette | Owned by: | assefa |
---|---|---|---|
Priority: | normal | Milestone: | 6.0 release |
Component: | WFS Server | Version: | 5.0 |
Severity: | normal | Keywords: | OGC, Cite, TEAM, WFS 1.0.0 |
Cc: |
Description
The following code in msWFSDescribeFeatureType() was introduced in r2435 and is intended to strip namespaces in the list of typenames provided by the caller but it seems to have a few problems listed below:
layers = msStringSplit(paramsObj->pszTypeName, ',', &numlayers); if (numlayers > 0) { /* strip namespace if there is one :ex TYPENAME=cdf:Other */ tokens = msStringSplit(layers[0], ':', &n); if (tokens && n==2 && msGetLayerIndex(map, layers[0]) < 0) { msFreeCharArray(tokens, n); tokens = NULL; for (i=0; i<numlayers; i++) { tokens = msStringSplit(layers[i], ':', &n); if (tokens && n==2) { free(layers[i]); layers[i] = strdup(tokens[1]); } if (tokens) msFreeCharArray(tokens, n); tokens = NULL; } } if (tokens) msFreeCharArray(tokens, n); }
1- We check only layers[0] to decide if we are going to strip namespaces. What if layer[0] doesn't contain a namespace and layer[1] does contain a namespace? Should we not strip namespaces anyway?
2- I guess the conclusion from the question above is that we should loop on all layers and try each layer name with msGetLayerIndex() and if they fail try removing the namespace prefix? Would that be better?
3- Doing a msStringSplit() to look for a ':' delimiter is expensive. Would it not be better to use " if (strchr( string, ':') != NULL) ... " instead?
4- Since we have very similar code in msWFSGetFeature(), should we not move that to a function and reuse it?
Assefa, what would you think of using the following (untested code) and moving it to a function?
layers = msStringSplit(paramsObj->pszTypeName, ',', &numlayers); for (i=0; i<numlayers; i++) { if ( msGetLayerIndex(map, layers[i]) < 0 ) { /* Try stripping namespace if there is one ex: TYPENAME=cdf:Other */ const char *basename; basename = strchr(layers[i], ':'); if (basename && basename[0] != '\0' && msGetLayerIndex(map, basename) >= 0) { free(layers[i]); layers[i] = strdup(basename); } } }
Change History (4)
comment:1 by , 17 years ago
Status: | new → assigned |
---|
comment:2 by , 17 years ago
Milestone: | 5.0 release → 5.2 release |
---|
comment:3 by , 17 years ago
Keywords: | OGC Cite TEAM WFS 1.0.0 added |
---|
comment:4 by , 15 years ago
Milestone: | 5.6 release → 6.0 release |
---|
daniel, all 4 comments make sense. I will do the corrections before beta3.