Cleanup code that strips namespaces in msWFSDescribeFeatureType()

[tbonfort]

Reporter: dmorissette Date: 2007/08/02 - 20:16 The following code in msWFSDescribeFeatureType() was introduced in 1ca8d3601abf89681eb2e987bc3dd11f4796baaf (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);
        }
      }
    }

[tbonfort]

Author: assefa Date: 2007/08/02 - 21:20 daniel, all 4 comments make sense. I will do the corrections before beta3.

[Schpidi]

This is an automated comment

This issue has been closed due to lack of activity. This doesn't mean the issue is invalid, it simply got no attention within the last year. Please reopen if still valid.