Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetFeatureInfo format list in GetCapabilities not respected #9007

Closed
1 of 6 tasks
Tracked by #8167
tdipisa opened this issue Mar 8, 2023 · 8 comments · Fixed by #9126 or #9134
Closed
1 of 6 tasks
Tracked by #8167

GetFeatureInfo format list in GetCapabilities not respected #9007

tdipisa opened this issue Mar 8, 2023 · 8 comments · Fixed by #9126 or #9134

Comments

@tdipisa
Copy link
Member

tdipisa commented Mar 8, 2023

Description

Same issue as #301 for GetFeatureInfo, whatever the formats proposed by the server mapstore assumes text/html and application/json are available.

by default mapserver has:

<GetFeatureInfo>
<Format>text/plain</Format>
<Format>application/vnd.ogc.gml</Format>

sure, i can add formats to that list by properly configuring mapserver, but only the formats supported by the server should be listed in the featureinfo layer dialog. Sure, its probably the default that's supported by geoserver, but still some interoperability tests would be nice.

What kind of improvement you want to add? (check one with "x", remove the others)

  • Minor changes to existing features
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Other useful information

Coming from georchestra/mapstore2-georchestra#302

@allyoucanmap
Copy link
Contributor

@landryb thanks for the tests done in #9054
I'm going to link the comment here to keep track of them:

#9054 (comment)
#9054 (comment)

@allyoucanmap
Copy link
Contributor

@tdipisa

The PR #9054 has already improved the get feature info formats parsing from a WMS capabilities but there are still some missing parts to complete this issue:

  1. There are possible problems using the global get feature info settings if a WMS service does not contains all the formats supported by MapStore
  2. After saving a map the get info formats are not persisted

Proposal

  • [Point 1.] We could add a check inside the recordToLayer function of the WMS catalog API to include the featureInfo property to a layer in case the supported formats does not match all the one supported by MapStore, in this way we ensure the layer is not affected by the global settings feature info selection. We will have three cases:

    • the capabilities supports all the formats supported by MapStore
    • the capabilities supports some of the formats supported by MapStore -> We could create a function that compare the list of supported formats from MapStore with the one from the capabilities and select the first one supported. The function should return null if the capabilities supports all the formats supported by MapStore
    /*
    featureInfo could be one of the following formats: "TEXT", "HTML" or "PROPERTIES"
    featureInfo: {
        format: "TEXT" // "TEXT", "HTML" or "PROPERTIES"
    }
    */
    const featureInfo = getFeatureInfoProperty(supportedGetFeatureInfoFormats);
    return {
        // ...other properties of the layer,
        ...(featureInfo && { featureInfo })
    }
    • the capabilities does not support any of the format supported by MapStore -> We should apply the HIDDEN format so the layer will not be queryable (we could use the same function described in the previous case)
    featureInfo: {
        format: "HIDDEN"
    }
  • [Point 2.] We have two options to solve this problem:

    • a) We could save the infoFormats in the layer options saved in the map configuration
    • b) We could introduce a refresh system including a check to request the list of info formats every time the settings tab is selected and the layer does not contain the infoFormats property (similar behavior of the GetMap formats in the settings)

    Note: The a) option should be the easiest one to implement


The problems mentioned in this comment #9007 (comment) should be investigated in different issues

@tdipisa
Copy link
Member Author

tdipisa commented Apr 5, 2023

Thank you @allyoucanmap for the above. I've checked it and discussed briefly with @offtherailz. @offtherailz will review today more in details.

@offtherailz
Copy link
Member

offtherailz commented Apr 6, 2023

The ideas are fine for me. Everything looks good.

here my comments and suggestions:

Point 1:

  • For WMS we effectively have the format list in the capabilities. I should keep this behivior as much similar as the getMap system for simpicity. I agree with the auto-selection algorithm of the format, but I whould replace the auto-selection in case of "some" with a selector in advanced options, similar to the format selector for image. (the default value should be saved in the catalog). So the user can configure the feature info format applied by default for the given catalog, and change it in the layer (Point 2).
  • For CSW, we do not have the format list from capabilities. So we can only select the map default. Doing a Capabilities during the add-layer is not suggested in this case. Eventually the user can configure it in point 2.

Point 2:

  • For consistency with the image format, I'd choose the "b" point, so this way the user can adjust feature info format in a second time refreshing and selecting the format. The options not available should be disabled in the selector ( so template and properties disabled if JSON format not available).

@tdipisa
Copy link
Member Author

tdipisa commented Apr 6, 2023

For WMS we effectively have the format list in the capabilities. I should keep this behivior as much similar as the getMap system for simpicity. I agree with the auto-selection algorithm of the format, but I whould replace the auto-selection in case of "some" with a selector in advanced options, similar to the format selector for image. (the default value should be saved in the catalog). So the user can configure the feature info format applied by default for the given catalog, and change it in the layer (Point 2).

I agree.

For consistency with the image format, I'd choose the "b" point, so this way the user can adjust feature info format in a second time refreshing and selecting the format. The options not available should be disabled in the selector ( so template and properties disabled if JSON format not available).

I agree with that, but I would maintain the current UI. Therefore this means to perform a getcapa when clicking in the Identify tab of layer settings

@allyoucanmap
Copy link
Contributor

allyoucanmap commented Apr 12, 2023

Development tasks summary

Point 1

Improvement on the catalog advanced settings needs following changes:

<FormGroup style={advancedRasterSettingsStyles}>
    <Col xs={6}>
        <ControlLabel><Message msgId="catalog.infoFormat.label" /></ControlLabel>
    </Col >
    <Col xs={6} style={{marginBottom: '5px', display: 'flex', alignItems: 'center' }}>
        <div style={{ flex: 1 }}>
            <Select
                isLoading={props.formatsLoading}
                onOpen={() => onFormatOptionsFetch(service.url)}
                value={service && service.infoFormat}
                clearable
                options={props.formatsLoading ? [] : infoFormatOptions.map((format) => ({ value: format, label: format }))}
                onChange={event => onChangeServiceProperty("infoFormat", event && event.value)} />
        </div>
        <Button
            disabled={props.formatsLoading}
            tooltipId="catalog.format.refresh"
            className="square-button-md no-border"
            onClick={() => onFormatOptionsFetch(service.url, true)}
            key="format-refresh">
            <Glyphicon glyph="refresh" />
        </Button>
    </Col >
</FormGroup>
  • we need to pass infoFormatOptions inside the CatalogServiceEditor to pass the to AdvancedSettings component. The infoFormatOptions prop is already connected but not passed to this components.
infoFormatOptions={infoFormatOptions}
const {
    format: defaultFormat,
    infoFormat,
    localizedLayerStyles,
    allowUnsecureLayers,
    autoSetVisibilityLimits,
    layerOptions
} = service || {};

const featureInfo = infoFormat && INFO_FORMATS_BY_MIME_TYPE[infoFormat]
    ? { format: INFO_FORMATS_BY_MIME_TYPE[infoFormat] }
    : null;

return {
    // ...,
    ...(featureInfo && { featureInfo }),
    // ...
}

If we have the select we don't need "magic" computation to detect the default feature info format and it will reduce the complexity

image

Point 2

Improvement of the FeatureInfo component to requests info formats if they are missing in the layer (element) configuration as soon as the component mount in the panel. This will need a loading phase. Sample of code (to be verify in development):

state = {
    loading: false
}

componentDidMount() {
    if (!this.props.element.infoFormats || this.props.element.infoFormats?.length === 0) {
        this.setState({ loading: true });
        getSupportedFormat(this.props.element.url, true)
            .then(({ infoFormats }) => {
                this.props.onChange("infoFormats", infoFormats);
                this.setState({ loading: false });
            })
            .catch(() => {
                this.setState({ loading: false });
            });
    }
}

open question raised during the discussion with @landryb :

Could we also save the infoFormats in the map configuration due the fact that the info formats are not updated often in the capabilities, in this way we could avoid the GetCapabilities request?

@tdipisa
Copy link
Member Author

tdipisa commented Apr 12, 2023

@allyoucanmap @landryb

open question raised during the discussion with @landryb :

Could we also save the infoFormats in the map configuration due the fact that the info formats are not updated often in the capabilities, in this way we could avoid the GetCapabilities request?

I wouldn't like to complicate things right now by introducing changes that need to be better discussed. I'm open to change the current behavior by saving in the map state both infoFormats and imageFormats but as part of a separated issue and for now maintain the current policy: a request to load the infoFormats when opening the Identify tab.

For the rest, all is ok for me. Thank you so much.

landryb added a commit to landryb/MapStore2 that referenced this issue Apr 24, 2023
landryb added a commit to landryb/MapStore2 that referenced this issue Apr 24, 2023
otherwise everything blows because infoFormatOptions isn't known ?
landryb added a commit to landryb/MapStore2 that referenced this issue Apr 24, 2023
…ions-it#9007)

convert the infoFormat in the featureInfo layer property by using the
INFO_FORMATS_BY_MIME_TYPE utils.
@landryb
Copy link
Collaborator

landryb commented Apr 24, 2023

return {
    // ...,
    ...(featureInfo && { featureInfo }),
    // ...
}

hi @allyoucanmap , i've resumed work on this (cf https://github.com/landryb/MapStore2/commits/wip/9007), but i'm puzzled by this bit. Shouldn't that rather be a new property in the return value of recordToLayer cf

? if so that's not a return.. but rather an assignation like featureInfo: featureInfo ? that's what i did in 54d3c48

will do more tests, but with that i can select a default infoformat for a service, and layers added from that service use it upon gfi requests. If i go look at the layer properties, in the feature info tab, the default infoformat for the service is correctly highlighted.

allyoucanmap pushed a commit that referenced this issue Apr 27, 2023
… & IGN french services (#9134)

* make sure styles queryparam is defined for GFI requests

improves interoperability with mapserver 8, without a style params, GFI requests
fail with:
"msWMSLoadGetMapParams(): WMS server error. Missing required parameter STYLES.
Note to service administrators: defining the "wms_allow_getmap_without_styles"
"true" MAP.WEB.METADATA item will disable this check (backward compatibility
with behaviour of MapServer < 8.0)"

* send the format queryparam in GFI requests too

some WMS servers (like IGN national services in france) complain about a
missing FORMAT param even if it's not made mandatory nor specified in the spec.
allyoucanmap pushed a commit that referenced this issue Apr 27, 2023
* when displaying layer properties, get supported infoFormats if not already known

* display a spinner if we're loading/parsing supported infoFormats

* add a new infoFormat selector in advanced raster settings (#9007)

fetches capabilities upon open

* define an empty infoFormatOptions (#9007)

otherwise everything blows because infoFormatOptions isn't known ?

* pass infoFormatOptions down to AdvancedSettings in CatalogServiceEditor (#9007)

* initialize layer featureInfo from service infoFormat if set (#9007)

convert the infoFormat in the featureInfo layer property by using the
INFO_FORMATS_BY_MIME_TYPE utils.

* add missing semicolon to please eslint

* add missing imports to please eslint

* add new test for infoFormat onChangeServiceProperty

* fix other tests after infoFormat addition

* improve onChangeServiceProperty layerOption tileSize test by testing the actual value

* fix FeatureInfo-test by making sure url is defined in componentDidMount

* default INFO_FORMATS_BY_MIME_TYPE[application/json] to PROPERTIES

this way PROPERTIES is highlighted in the featureinfo tab of layer properties
if 'application/json' was selected as the default infoFormat for the source
catalog entry.

* apply some inline style to the Loader displayed while populating featureinfo tab
@ElenaGallo ElenaGallo self-assigned this Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment