Opened 17 years ago

Last modified 14 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 assefa, 17 years ago

Status: newassigned

daniel, all 4 comments make sense. I will do the corrections before beta3.

comment:2 by assefa, 17 years ago

Milestone: 5.0 release5.2 release

comment:3 by nsavard, 16 years ago

Keywords: OGC Cite TEAM WFS 1.0.0 added

comment:4 by assefa, 14 years ago

Milestone: 5.6 release6.0 release
Note: See TracTickets for help on using tickets.