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

Add an action route that returns the fields information for WFS layers #670

Conversation

mingfeng
Copy link
Contributor

@mingfeng mingfeng commented Dec 4, 2020

The result is structured as:

{
    "attributes": {
        "attr-1": "string",
        "attr-2": "number",
        ...
        "attr-n": "unknown",
    },
    "geometryField": "geometry",
    "locale": {
        "en": {
            "attr-1: "attr-1-en"
            ...
        },
        "fi": {
            "attr-1": "attr-1-fi"
            ...
        },
        ...
    }
}

@mingfeng mingfeng force-pushed the feature/add-get-wfs-layer-fields-action-route branch 6 times, most recently from 64e4567 to 3c7b125 Compare December 8, 2020 09:46
@mingfeng mingfeng marked this pull request as ready for review December 8, 2020 10:19
@mingfeng mingfeng force-pushed the feature/add-get-wfs-layer-fields-action-route branch from 3c7b125 to ea107d9 Compare December 8, 2020 10:23

@Override
public void handleAction(ActionParameters params) throws ActionException {
final int layerId = ConversionHelper.getInt(params.getRequiredParam(PARAM_LAYER_ID), -1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final int layerId = ConversionHelper.getInt(params.getRequiredParam(PARAM_LAYER_ID), -1);
final int layerId = params.getRequiredParamInt(PARAM_LAYER_ID)

}

private OskariLayer getLayer(int layerId) throws ActionException {
final OskariLayer layer = layerService.find(layerId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run this through the PermissionHelper as in https://github.com/oskariorg/oskari-server/blob/master/control-base/src/main/java/fi/nls/oskari/control/layer/GetLayerTileHandler.java#L67

The helper will throw a ActionDeniedException if the user doesn't have permission to view the layer -> they shouldn't get to see the attributes/configurations we have on that layer either.

if (layer == null) {
throw new ActionException("Layer not found: " + layerId);
}
return layer;
Copy link
Member

@ZakarFin ZakarFin Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return layer;
if (!OskariLayer.TYPE_WFS.equals(layer.getType()) {
throw new ActionParamsException("Only WFS supported. Wrong layer type: " + layer.getType());
}
return layer;

*
*/
private static JSONObject getCollectionFields(OskariLayer layer) throws ServiceException {
String url = layer.getUrl() + "collections/" + layer.getName() + "/items";
Copy link
Member

@ZakarFin ZakarFin Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use https://github.com/oskariorg/oskari-server/blob/master/service-wfs3/src/main/java/org/oskari/service/wfs3/OskariWFS3Client.java#L229 OskariWFS3Client.getItemsPath(layer.getUrl(), layer.getName()); (make the static method public on the client) so it's easier to see where OGC API Features services are called. I don't think we should use the existing client for getting the features since it's probably doing too much for what we need here. We could use OskariWFS3Client.validateResponse(conn, CONTENT_TYPE_GEOJSON); though for sanity checking the response.

while (attributeNames.hasNext()) {
String attributeName = (String) attributeNames.next();
String attributeType = propertyTypes.getString(attributeName);
attributeType = attributeType.split(":")[1]; // remove xml namespace prefix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make sure there is a ':' character included and not assume there is. Most probably have it but there's always the chance that XML default namespace is set based on the featuretype.

OskariLayerService mockLayerService = mock(OskariLayerServiceMybatisImpl.class);
OskariLayer wfsLayer = new OskariLayer();
wfsLayer.setType("WFS");
final String rawJsonStr = "{\"data\": {\"locale\": {\"fi\": {\"field-en\": \"field-fi\"}}}}";
Copy link
Member

@ZakarFin ZakarFin Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"field-en": "field-fi" is a bit misleading here. Rename to something like "test-attribute": "label for fi" or similar.

private static void mockOskariComponentManager() throws JSONException {
OskariLayerService mockLayerService = getMockLayerService();
PowerMockito.mockStatic(OskariComponentManager.class);
when(OskariComponentManager.getComponentOfType(OskariLayerService.class)).thenReturn(mockLayerService);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we've mostly used setters on handlers for setting up mock services with init() checking if the services have been set before overwriting them with values from the component manager. But this is ok since it looks like it's not leaking the mocks between tests.


private OskariLayer getWFSLayer(String version) {
OskariLayer layer = new OskariLayer();
layer.setType("WFS");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
layer.setType("WFS");
layer.setType(OskariLayer.TYPE_WFS);

@mingfeng mingfeng force-pushed the feature/add-get-wfs-layer-fields-action-route branch 2 times, most recently from e61eabb to 60657d8 Compare December 10, 2020 11:07
The result is structured as:

{
    "attributes": {
        "attr-1": "string",
        "attr-2": "number",
        ...
        "attr-n": "unknown",
    },
    "geometryField": "geometry",
    "locale": {
        "en": {
            "attr-1: "attr-1-en"
            ...
        },
        "fi": {
            "attr-1": "attr-1-fi"
            ...
        },
        ...
    }
}
@mingfeng mingfeng force-pushed the feature/add-get-wfs-layer-fields-action-route branch from 60657d8 to 3ab105e Compare December 10, 2020 12:38
@sonarcloud
Copy link

sonarcloud bot commented Dec 10, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ZakarFin ZakarFin added this to the 2.2.0 milestone Dec 10, 2020
@ZakarFin ZakarFin merged commit 30b1508 into oskariorg:develop Dec 10, 2020
@mingfeng mingfeng deleted the feature/add-get-wfs-layer-fields-action-route branch December 10, 2020 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants