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 graph support for ServiceStore #580

Merged
merged 13 commits into from
Oct 26, 2021
Merged

Conversation

gayathrir11
Copy link
Contributor

@gayathrir11 gayathrir11 commented Oct 18, 2021

Summary

Closes #549

How did you test this change?

Tested the change by compiling the grammar in text mode.
Grammar test needs to be skipped for now. There is an extra space being returned for service parameters from engine side. Needs a fix on engine.
Grammar used for testing

###ExternalFormat
SchemaSet test::tradeSchema
{
  format: FlatData;
  schemas: [
    {
      content: 'section trade: DelimitedWithHeadings\n{\n  scope.untilEof;\n  delimiter: \',\';\n  nullString: \'\';\n\n  Record\n  {\n    Product           : STRING;\n    Quantity          : INTEGER;\n    \'Trade Time\'      : DATETIME;\n    Price             : DECIMAL;\n    \'Price Ccy\'       : STRING;\n   \'Settlement Ccy\'  : STRING(optional);\n    \'Settlement Rate\' : DECIMAL(optional);\n   \'Settlement Date\' : DATE;\n   \'Confirmed At\'    : DATETIME(optional);\n   \'Expiry Date\'     : DATE(optional);\n   \'Executions\'      : INTEGER(optional);\n }\n}';
    }
  ];
}

Binding test::gen::TestBinding
{
  schemaSet: test::tradeSchema;
  contentType: 'application/x.flatdata';
  modelIncludes: [
    test::gen::Trade
  ];
}


###Pure
Class test::gen::Trade
{
  product: String[1];
  quantity: Integer[1];
  tradeTime: DateTime[1];
  price: Float[1];
  priceCcy: String[1];
  settlementCcy: String[0..1];
  settlementRate: Float[0..1];
  settlementDate: StrictDate[1];
  confirmedAt: DateTime[0..1];
  expiryDate: StrictDate[0..1];
  executions: Integer[0..1];
}


###ServiceStore
ServiceStore test::testServiceStoreCompilationWithSingleService1
(
  Service TestService
  (
    path : '/testService';
    method : GET;
    parameters :
    (
      serializationFormat : String ( location = query )
    );
    response : [test::gen::Trade <- test::gen::TestBinding];
    security : [];
  )
)

ServiceStore test::ServiceStore1
(
  ServiceGroup TestServiceGroup
  (
    path : '/testServices';

    Service TestService1
    (
      path : '/testService1';
      method : GET;
      parameters :
      (
        param : String ( location = query ),
        param2 : Integer ( location = query )
      );
      response : test::gen::Trade <- test::gen::TestBinding;
      security : [];
    )
    Service TestService2
    (
      path : '/testService2';
      method : GET;
      parameters :
      (
        param1 : Boolean ( location = query )
      );
      response : test::gen::Trade <- test::gen::TestBinding;
      security : [];
    )
  )
)


###Mapping
Mapping test::mapping
(
  *test::gen::Trade: ServiceStore
  {
    ~service [test::testServiceStoreCompilationWithSingleService1] TestService
    (
      ~paramMapping
      (
        serializationFormat : 'CSV'
      )
    )
  }
)


###Connection
ServiceStoreConnection simple::serviceStoreConnection
{
  store: test::testServiceStoreCompilationWithSingleService1;
  baseUrl: 'http://baseUrl';
}

@changeset-bot
Copy link

changeset-bot bot commented Oct 18, 2021

🦋 Changeset detected

Latest commit: c67bd3f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
@finos/legend-graph Minor
@finos/legend-extension-external-store-service Minor
@finos/legend-extension-dsl-serializer Patch
@finos/legend-extension-dsl-data-space Patch
@finos/legend-extension-dsl-diagram Patch
@finos/legend-extension-dsl-text Patch
@finos/legend-studio-app Patch
@finos/legend-query-app Patch
@finos/legend-manual-tests Patch
@finos/legend-studio Minor
@finos/legend-application Patch
@finos/legend-extension-external-format-json-schema Patch
@finos/legend-query Patch
@finos/legend-studio-extension-query-builder Patch
@finos/legend-studio-deployment Patch
@finos/legend-query-deployment Patch
@finos/legend-studio-extension-management-toolkit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@finos-cla-bot finos-cla-bot bot added the cla-present CLA Signed label Oct 18, 2021
@gayathrir11 gayathrir11 changed the title Service store Add graph support for DSL ServiceStore Oct 18, 2021
@akphi
Copy link
Contributor

akphi commented Oct 24, 2021

@gayathrir11 just a general feedback, I think next time, for big changes like this, let's split it over a few different PRs. At least I can see 3 here:

  • Remove the old ServiceStore
  • Introduce extension mechanism for class mapping
  • Add new ServiceStore

But all in all, this is great work! Thank you so much!

@gayathrir11 gayathrir11 changed the title Add graph support for DSL ServiceStore Add graph support for ServiceStore Oct 25, 2021
@gayathrir11
Copy link
Contributor Author

@akphi I have a concern. When I click on edit in textmode for the new classMapping I added it throws an error. When I close the error it opens textMode. Don't you think it will confuse the user or make the UI state inconsistent. Attaching a video for reference

Legend.Studio.-.Google.Chrome.2021-10-25.12-16-43.mp4

@gayathrir11
Copy link
Contributor Author

@akphi I have a concern. When I click on edit in textmode for the new classMapping I added it throws an error. When I close the error it opens textMode. Don't you think it will confuse the user or make the UI state inconsistent. Attaching a video for reference
Found the reason for the issue. We are making a call for setImplementation.accept_SetImplementationVisitor in InstanceSetImplementationEditor.tsx where we are passing new MappingElementDecorationCleaner() as a parameter. It is calling all the functions inside this accept_SetImplementationVisitor and popping the errors. Tested this by commenting out that line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-present CLA Signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Add graph support for ServiceStore
3 participants