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

Owc fixes #98

Merged
merged 15 commits into from
Sep 23, 2021
Merged

Owc fixes #98

merged 15 commits into from
Sep 23, 2021

Conversation

MichaelLangbein
Copy link
Collaborator

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

This PR fixes a few errors in the current implementation of OwcJsonService

  • createVectorLayerFromOffering didn't work properly with WFS'es
  • VectorLayers did not get the correct description based on an OWC's abstract field

What is the new behavior?

Errors have been fixed.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Is it part of one/more packages and which?

  • @dlr-eoc/services-ogc

@MichaelLangbein
Copy link
Collaborator Author

@boeckMt : should we create another next release with this PR? With that, we could remove the projects dir from CoastalX.

@boeckMt
Copy link
Member

boeckMt commented Sep 20, 2021

There are further Issues in creating layers from Offerings.
We often used offering.operations[0]

Then offering.contents[0].content

For WFS we maybe also should check if the minimum required parameters are in the href

@boeckMt
Copy link
Member

boeckMt commented Sep 21, 2021

Update Types to fit the OWS Context GeoJSON Standard #97

e.g. values was changed to use period in display and not providing it like Geoserver

and now it can also be

{
  "name": "time",
  "values": "2003-01-01T00:00:00.000Z/2018-01-01T00:00:00.000Z",
  "units": "ISO8601",
  "display": {
     "default": "2003-01-01T00:00:00Z",
     "period": "P1Y"
   }
}

update https://github.com/dlr-eoc/ukis-frontend-libraries/blob/v7.3.2-next.3/projects/services-ogc/src/lib/owc/owc-json.service.ts#L222

@boeckMt
Copy link
Member

boeckMt commented Sep 21, 2021

Fix createLayerFromOffering https://github.com/dlr-eoc/ukis-frontend-libraries/blob/v7.3.2-next.3/projects/services-ogc/src/lib/owc/owc-json.service.ts#L454

createLayerFromOffering(offering: IOwsOffering, resource: IOwsResource, context: IOwsContext, targetProjection: string): Observable<Layer> {
    const layerType = this.getLayertypeFromOfferingCode(offering);
    if (isRasterLayertype(layerType)) {
      return this.createRasterLayerFromOffering(offering, resource, context, targetProjection);
    } else if (isVectorLayertype(layerType)) {
      return this.createVectorLayerFromOffering(offering, resource, context);
    } else if (layerType === TmsLayertype) {
      return this.createTmsLayerFromOffering(offering, resource, context, targetProjection);
    } else {
     //  console.error(`This type of service (${layerType}) has not been implemented yet.`);
      console.warn(`This type of service (${layerType}) has not been implemented yet.`);
      return of(<Layer>{});
    }
  }

Try to prevent using console.error and return an null/empty Observable here, otherwise the whole application stops working if someone uses a context with an unknown offering.code like "http://schemas.opengis.net/wms/1.1.1",

Langbein, Michael added 2 commits September 21, 2021 20:48
… offering.content or offering.operation

instead of blindly choosing the first in list
@MichaelLangbein
Copy link
Collaborator Author

Just to clarify:

Here, do you mean that we should replace the type values: string; with a template-literal?

@boeckMt
Copy link
Member

boeckMt commented Sep 22, 2021

Just to clarify:

Here, do you mean that we should replace the type values: string; with a template-literal?

No what I wanted to say is that we have to check if there is a period on the values
e.g.

  • 1984-01-01T00:00:00.000Z/1989-12-31T23:59:59.000Z
    or it could be like
  • 1984-01-01T00:00:00.000Z/1989-12-31T23:59:59.000Z/P1Y

We have such a function but it is not used in in the owc service https://github.com/dlr-eoc/ukis-frontend-libraries/blob/v7.3.2-next.3/projects/services-ogc/src/lib/owc/owc-json.service.ts#L199


But your proposal is a good Idea. We could type it e.g.

type isoInterval = `${string}/${string}`;
type intervalPeriod = `${isoInterval}/P${string}`;

export interface IEocOwsResourceDimension {
  name: 'time' | 'elevation';
  /**
   * For time:
   *  - '1984-01-01T00:00:00.000Z,1990-01-01T00:00:00.000Z,1995-01-01T00:00:00.000Z'
   *  - '2000-09-01T00:00:00.000Z/2017-08-31T00:00:00.000Z/P1D'
   */
  values: `${string},` | isoInterval | intervalPeriod;
 ...
}

It is not catching all cases but a bit more typesave.

https://www.typescriptlang.org/play?#code/C4TwDgpgBAlgzgewJIDtgQE4DcCGAbKAXigAMASAbzmAxhQHMBfAekutocZIG4AoUSLDSZceAAqYYCACZFSleMmHZ8LMWxp0mPfuGiiArhDhzyVTZwA0GjtqgAfWIlToVBR3VeiJtGX168AMYIKNRQALYgLiL4AFxOSl74RADk6NTM6cApQSFhkdFuPlLS8Z4x4pIyqVmZxsDMYlk5waHAEVHK3lXSAExlXfjF1YRp9Y3NuW0dAGr4RnAAjPGGxjX1LXntkXN4C-1QqyajWZab0zvzxgDMK1fHYxlNG1P5ILsLACx3e2vEjw1akCXkA

@@ -461,6 +469,7 @@ export class OwcJsonService {
return this.createTmsLayerFromOffering(offering, resource, context, targetProjection);
} else {
console.error(`This type of service (${layerType}) has not been implemented yet.`);
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 here it would be good if you also log the resource or offering so that the use knows what is causing this problem

@MichaelLangbein
Copy link
Collaborator Author

No what I wanted to say is that we have to check if there is a period on the values
e.g.

Not quite sure if I'm getting this right, but doesn't getTimeDimensions already check for the presence of a period in a IEocOwsResourceDimension?

I'm not certain if convertOwcTimeToIsoTimeAndPeriodicity is being used anywhere at all. It might be obsolete by now and replaced by getTimeDimensions. In that case it might make sense to remove this function, since we're moving to a new major version, anyway.

@boeckMt
Copy link
Member

boeckMt commented Sep 22, 2021

I'm not certain if convertOwcTimeToIsoTimeAndPeriodicity is being used anywhere at all. It might be obsolete by now and > replaced by getTimeDimensions. In that case it might make sense to remove this function, since we're moving to a new major > version, anyway.

Ah we used it in an earlier version of getTimeValueFromDimensions https://github.com/dlr-eoc/ukis-frontend-libraries/blob/v7.3.0/projects/services-ogc/src/lib/owc/owc-json.service.ts#L225 and it was not reused or removed.

But it looks like it is replaced by other functions, so you can remove it (but list it in the Breaking Changes please).

The only thing which is missing is, if period is null https://github.com/dlr-eoc/ukis-frontend-libraries/blob/v7.3.2-next.3/projects/services-ogc/src/lib/owc/owc-json.service.ts#L305 then it should not be added to dim.display.


And parsing of the operation.href was accidentally removed

https://github.com/dlr-eoc/ukis-frontend-libraries/blob/owcFixes/projects/services-ogc/src/lib/owc/owc-json.service.ts#L732

const getMapOperation = offering.operations.find(o => o.code === 'GetMap');

- const urlParams = getMapOperation.href;
+ const urlParams = this.getJsonFromUri(getMapOperation.href);

And can you pleas change https://github.com/dlr-eoc/ukis-frontend-libraries/blob/owcFixes/projects/services-ogc/src/lib/owc/owc-json.service.ts#L740

 const params: IWmsParams = {
        LAYERS: urlParams['LAYERS'],
        FORMAT: urlParams['FORMAT'],
        TIME: urlParams['TIME'],
        VERSION: urlParams['VERSION'],
        TILED: urlParams['TILED'],
        TRANSPARENT: true,
        STYLES: defaultStyle
      };

to

const params: IWmsParams = {
        LAYERS: urlParams['LAYERS'],
        TRANSPARENT: true,
        STYLES: defaultStyle
      };
      if (urlParams['FORMAT']) {
        params.FORMAT = urlParams['FORMAT'];
      }
      if (urlParams['TIME']) {
        params.TIME = params.urlParams['TIME'];
      }
      if (urlParams['VERSION']) {
        params.VERSION = urlParams['VERSION'];
      }
      if (urlParams['TILED']) {
        params.TILED = urlParams['TILED'];
      }

@MichaelLangbein
Copy link
Collaborator Author

All right, all comments are handled and all tests & build are working. Thanks a lot for the help, @boeckMt ! If nothing else is missing, I could now push new tags to the repo to trigger the pre-release. Is that alright with you?

Copy link
Member

@boeckMt boeckMt left a comment

Choose a reason for hiding this comment

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

Looks like everything is good now, great work.
Yes you can make the pre version.

@MichaelLangbein MichaelLangbein merged commit 20e884a into master Sep 23, 2021
boeckMt added a commit that referenced this pull request Sep 27, 2021
@boeckMt boeckMt deleted the owcFixes branch October 6, 2021 12:08
@boeckMt boeckMt restored the owcFixes branch October 19, 2021 07:00
@boeckMt boeckMt deleted the owcFixes branch October 19, 2021 07:00
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.

None yet

2 participants