SAP / open-ux-odata

Enable community collaboration to jointly promote and facilitate best in class framework and tooling capabilities when working with OData services.
Apache License 2.0
51 stars 10 forks source link

BUG - annotation-converter exception when resolving references #847

Closed schreckstefan closed 2 weeks ago

schreckstefan commented 3 weeks ago

Related Feature

Support of freestyle apps by Fiori Tools

Description

While testing, I generated a test app with the app generator. I used the basic template, but an ALP mock service for it. When trying to open the Page Map for it, it fails with an exception.

image

Reason is an exception at sap-ux/annotation-converter:

Screenshot 2024-08-23 at 10 52 12

As a result, the schemas and configs cannot be generated by sap/ux-specification:

image

Steps to Reproduce

Steps to reproduce the behavior:

  1. Open VSCode and install the SAP Fiori Tools - Extension Pack
  2. Open the test app basicwithservice2208-2024-08-230855.zip
  3. Use command "Fiori: Show Page Map"
  4. See error

Expected results

The given alis definitions look OK and should get resolved

Actual results

Exception

Version/Components/Environment

Add any other context about the problem here OS:

Root Cause Analysis

Problem

{describe the problem}

Fix

{describe the fix}

Why was it missed

{Some explanation why this issue might have been missed during normal development/testing cycle}

How can we avoid this

{if we don’t want to see this type of issues anymore what we should do to prevent}

nlunets commented 3 weeks ago

Hi @schreckstefan i just checked the part which are specific to the annotation converter and loading and processing this file works well on the latest version.

My assumption is that you are not calling this properly. Looking at the failing code and the error message i can only imagine that the references from your input type is a readonly variable, when we're expecting it not to be.

Parsing with edmx-parser (which is the standard implementation) and converting works fine. Please have a look on your side and close this issue when that's done :)

schreckstefan commented 2 weeks ago

Hi @nlunets , The coding did not change on our side since a while: converterOutput = convert(mergeParser(...parseResult)); Even if I change this to

let mergedParserResult = mergeParser(...parseResult);
converterOutput = convert(mergedParserResult);

I run into the same error.

nlunets commented 2 weeks ago

@schreckstefan the problem on your side occurs because there is no references in this file, while i assume all the other you tested had some.

schreckstefan commented 2 weeks ago

@nlunets Yes, you are right. The app was corrupted somehow. Does not appear with newly generated app. Let me close the issue - if I can reproduce in future, I will clarif ywith the app gen colleagues.

schreckstefan commented 2 weeks ago

Sorry @nlunets , But a new issue appeared which describes the same problem, so we have to find a solution for it. Could you please test the conversion of project2-2024-08-221334.zip? Question is why you need to manipulate the original file at all, by adding references, which is the Northwind model that looks consistent.

And finally the question is why my workaound, as describe in https://github.com/SAP/open-ux-odata/issues/847#issuecomment-2310075875, did not have any effect.

// Rest redacted as this included internal links

nlunets commented 2 weeks ago

The inputs are modified because if we need to writeback, we need the references we introduced. I can only reproduce the error if I deliberately freeze the input. I can try to make it so that even this case works ... let me see

nlunets commented 2 weeks ago

I've [pushed a new version https://github.com/SAP/open-ux-odata/commit/b1d4201a3b79832e5e3f5694c750f7eb3adc3f77 @sap-ux/annotation-converter@0.9.2 can you check ?

schreckstefan commented 2 weeks ago

You just updated dependencies? Ok, let me check. As mentioned, there had been some issues when I tried to apply 0.9.1 before. Will keep you informed.

nlunets commented 2 weeks ago

no i patched the code :)

schreckstefan commented 2 weeks ago

Looks much better with 0.9.2, only one unit test failing ... let me check that one

==> there obviously was a change in the representation of OData concat, in the converter result. I had to adjust our evaluation (guess Fiori Elements had to do the same, at the StableIdHelper?)

schreckstefan commented 2 weeks ago

@nlunets Confirmation: the new version works well, it solves the problem, and I can ship the consumption of the latest versions