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

[doc] Contribute a shape for model publication #4284

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gdaniel
Copy link
Contributor

@gdaniel gdaniel commented Dec 11, 2024

Pull request template

General purpose

What is the main goal of this pull request?

  • Bug fixes
  • New features
  • Documentation
  • Cleanup
  • Tests
  • Build / releng

Project management

  • Has the pull request been added to the relevant project and milestone? (Only if you know that your work is part of a specific iteration such as the current one)
  • Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
  • Have the relevant issues been added to the pull request?
  • Have the relevant labels been added to the issues? (area:, difficulty:, type:)
  • Have the relevant issues been added to the same project and milestone as the pull request?
  • Has the CHANGELOG.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc? (Including changes in the GraphQL API)
  • In case of a change with a visual impact, are there any screenshots in the CHANGELOG.adoc? For example in doc/screenshots/2022.5.0-my-new-feature.png

Architectural decision records (ADR)

  • Does the title of the commit contributing the ADR start with [doc]?
  • Are the ADRs mentioned in the relevant section of the CHANGELOG.adoc?

Dependencies

  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
  • Are the new dependencies justified in the CHANGELOG.adoc?

Frontend

This section is not relevant if your contribution does not come with changes to the frontend.

General purpose

  • Is the code properly tested? (Plain old JavaScript tests for business code and tests based on React Testing Library for the components)

Typing

We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).

  • Variables have a proper type
  • Functions’ arguments have a proper type
  • Functions’ return type are specified
  • Hooks are properly typed:
    • useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
    • useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
    • useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
    • useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
    • useState<STATE_TYPE>(…)
  • All components have a proper typing for their props
  • No useless optional chaining with ?. (if the GraphQL API specifies that a field cannot be null, do not treat it has potentially null for example)
  • Nullable values have a proper type (for example let diagram: Diagram | null = null;)

Backend

This section is not relevant if your contribution does not come with changes to the backend.

General purpose

  • Are all the event handlers tested?
  • Are the event processor tested?
  • Is the business code (services) tested?
  • Are diagram layout changes tested?

Architecture

  • Are data structure classes properly separated from behavioral classes?
  • Are all the relevant fields final?
  • Is any data structure mutable? If so, please write a comment indicating why
  • Are behavioral classes either stateless or side effect free?

Review

How to test this PR?

Please describe here the various use cases to test this pull request

  • Has the Kiwi TCMS test suite been updated with tests for this contribution?

Sirius Web users should be able to:

- Publish a model with a given version number
- Import a published with a given version number
Copy link
Contributor

Choose a reason for hiding this comment

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

a published with => a published model with


== Solution

The contextual menu of _Model_ elements in the explorer contains a new _Publish_ tool.
Copy link
Member

Choose a reason for hiding this comment

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

How could you be sure that a single model is self contained and does not have references to other models in the same editing context? What would happen if a model A has references to objects in a model B? It cannot be acceptable to publish a broken model with proxies. I don't see how we could realistically support the publication of anything other than the entire editing context in Sirius Web.

As a result, this menu should probably be on the project itself. If we go one step further, I don't see how we could consider this feature as "model publication" instead of "project dependencies"

Copy link
Member

@sbegaudeau sbegaudeau Dec 12, 2024

Choose a reason for hiding this comment

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

If we want to publish parts of a project or even better if we want to publish multiple artifacts for a project, then I'm not sure that Sirius Web can decide how to do it. Think about how we are exporting X maven artifacts (that may be preprocessed by the way) from one repository. It then creates a dependency tree that is used by the one using the dependency.

The only thing I think we can do is to export all models in a project because that's the only coherent unit of semantic data that we have.

Copy link
Member

Choose a reason for hiding this comment

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

After having thought about this a bit more, given the fact that we have multiple downstream projects (at least 3 I think) that have implemented something along those lines, I suspect that we can't decide on our own what publishing means (for example a validation step before the publication, some processing to transform the data, etc).

I suspect that we should be able to register "workflows" (think Github Workflows for Github Actions) which let specifiers define how to publish things for a project.

public interface IProjectWorkflow {
    /*
     * Can us the nature of the project to figure out if the workflow is relevant
     */
    boolean canHandle(UUID projectId);

     /*
      * I'm not providing the editing context here on purpose to let the specifier load the snapshot of the
      * data from the database in order NOT to block the editing context event processor.
      */
    void handle(UUID projectId, ...);
}

A project could have multiple workflows (like we do in our Github repository) and the end user could just select the workflow to run for the publication. We have to keep in mind that a workflow could take some time (a few milliseconds to a couple of hours). The workflow would explicitly decide to perform a publication (it may not publish anything at all). One could make a workflow to publish everything, or another one which would for example only publish the papaya::Component from a Papaya project and not other artifacts.

If you want to run a workflow using some variables as input (here a version, or a specific object) you may contribute a dedicated tool to execute the mutation triggering the workflow in your own application. Some downstream projects already have dedicated user interface accessible conditionally from specific objects in specific projects to start this. In such use case, all downstream projects would "just" share a common workflow execution system and publication area (think Github Packages).

Copy link
Member

Choose a reason for hiding this comment

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

So there are probably two shapes for this topic:

  • How to publish artifacts
  • How to consume artifacts (project dependencies, reference vs copy etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, if Sirius Web forces specifiers to publish entire editing contexts, which is, as you said, the only thing that makes sense in a vacuum, this may conflict with existing publication implementations. For example, I know some downstream application have a publication mechanism where only a part of a model is published, because the application ensures that this sub model does not depend on anything.

If we go for the workflow approach, should Sirius Web provide a specific table for that? I am not sure this is what you mean by publication area. If we have such table in Sirius Web we need to let downstream applications add information next to the published model (e.g. version number, date, etc). This could be their responsibility, but in this case they would have to do kind of a lot of work every time they want to re-use the publication mechanism of Sirius Web. Maybe we can provide some default publication data that can be easily reused?

I agree this could be split into two shapes.

Copy link
Member

Choose a reason for hiding this comment

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

If we go for the workflow approach, should Sirius Web provide a specific table for that? I am not sure this is what you mean by publication area.

I think that there be should somewhere (probably another page) where users could see what has been published by a project. It would just list the artifacts published and when they have been published. Using a simple "static" table in a dedicated page (something like /projects/:projectId/artifacts or something.

If we have such table in Sirius Web we need to let downstream applications add information next to the published model (e.g. version number, date, etc). This could be their responsibility, but in this case they would have to do kind of a lot of work every time they want to re-use the publication mechanism of Sirius Web. Maybe we can provide some default publication data that can be easily reused?

I was thinking about that and for example in Papyrus Web (since I worked on this one a tiny bit), they have the following properties:

  • version kind (development, release, major or custom)
  • version (can be computed from version kind)
  • date
  • author
  • comments
  • copyright
  • message

Some fields are computed from others or from the backend for example they can check the latest artifact published to figure out the previous version and offer to create a new upgraded version (using a new release version kind to automatically go from 1.2.3 to 1.3.0 for example).

I don't think that every downstream projects could use the same properties and even if all artifacts on an instance of Sirius Web could use the same properties. They would still be free to provide a custom user interface to publish models but we could provide a way for specifiers to add some properties and tell us how to display them.

Something like this:

package org.eclipse.sirius.web.domain.boundedcontexts.publication;

@Table("artifact")
public class Artifact extends AbstractValidatingAggregateRoot<Artifact> implements Persistable<UUID> {
    @Transient
    private boolean isNew;

    @Id
    private UUID id;

    private String version;

    private String description;

    @MappedCollection(idColumn = "artifact_id", keyColumn = "index")
    private List<ArtifactProperty> properties;

    private Instant createdOn;

    private Instant lastModifiedOn;
}

with something like this:

@Table("artifact_property")
public record ArtifactProperty(
    String name,
    String value
) {}

This way, if a downstream application wants to provide a custom user interface or just a custom workflow which would capture additional properties to be attached to the artifact published, they could tell us. On the frontend, we would only, for now display the properties in additional columns of the table. The field version and description would be mandatory (the description could be blank but at least it would be handled natively by Sirius Web) and additional fields could be contributed if a downstream project wants it. This part regarding custom properties of the artifact published would definitely be a cutting back that could be implemented later.

Comment on lines 26 to 29
This tool opens a pop-up that asks the user for a version number and comments.

**Note**: Sirius Web will not allow to publish a model with the a version number that matches an already published version of the model.
User will have to update the version of the model if they want to re-publish it.
Copy link
Member

Choose a reason for hiding this comment

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

So users should be able to view the list of versions already published in a dedicated page and (probably later) open them in read only to view what has been published.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The initial mockup we did for this popup displays the current version number and the latest one. I agree this could be improved to let the user see the list of all the published versions, potentially with the date of publication.

In any case, with the workflow approach, I think this is not a Sirius Web question anymore: the implementation of the workflow and the UI components that let the user provide the necessary variables to handle the publication will be defined in downstream applications. This is still a question for Studio publication, but I discuss it on the corresponding comment.

Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree, I still think Sirius Web should have a page listing artifacts published but a specific downstream application will propose its end users with the integration of those artifacts using a specific user interface.

Comment on lines 31 to 33
The _Upload model_ tool in the explorer is renamed to _Import model_, and the associated popup contains 2 tabs:
- The _Upload_ tab that performs the same action as it does now (upload an external file as a model)
- The _Published Models_ tab presents a list of published models (with their version number) the user can select to import in their project.
Copy link
Member

Choose a reason for hiding this comment

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

Keep both features separated because they really don't have the same behavior or lifecycle (and it's less expensive) and they will grow further apart.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, this is not a Sirius Web question anymore with the workflow approach. But I keep it in mind.


=== Scenario

1. User publishes a model
Copy link
Member

Choose a reason for hiding this comment

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

The user should see what has been published in the past too and probably the list of projects that are using each version that has been published.

- The user selects the _Published Models_ tab in the popup
- The published models are displayed in the tab, with their version number
- The user selects a published model and presses _Import_
- The model is imported at the root of the project
Copy link
Member

Choose a reason for hiding this comment

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

What does imported mean?

  • Copied and free to be modified?
  • Referenced and immutable?

We will need to let the end users should decide and only keep a link to the project using what has been published only in case of a reference.


This shape doesn't address the issue of upgrading/downgrading and imported model with a new version.
In the proposed solution, importing two different versions of the same model will create two models in the project (one for each version).
This behavior matches the current behavior of Sirius Web when importing the same external file two times.
Copy link
Member

Choose a reason for hiding this comment

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

And I don't think that this is what we want for most use cases. If I create a library, my first use case would be to let people include this library in their project without letting them manipulate it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By upgrading I meant importing a newer version of an imported model, and assume that Sirius Web will take care of replacing the older version with the newer one (including some semantic changes to make sure the models are still somewhat consistent).

I didn't want to address this issue in this shape, because the "somewhat consistent" part is tricky, and overriding an imported model with a newer/older version can be done in a different shape later.

@@ -0,0 +1,80 @@
= Add support for model publication

== Problem
Copy link
Member

@sbegaudeau sbegaudeau Dec 12, 2024

Choose a reason for hiding this comment

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

We definitely need to talk about studios in this PRs. This feature was THE next step in the management of studio in Sirius Web. We worked for weeks on this topic before deactivating everything and putting the code aside. We will need to be able to publish studio projects and let end users use only the published view/domain instead of all the view/domain found in the database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the previous comments, I assume this would be an implementation of a workflow tailored to Studios. To me this whole feature looks like a different shape if we go for a solution where Sirius Web only provides the workflow execution mechanism.

Import model popup, displayed when clicking on the _Import Model_ tool in the explorer toolbar.
The _Upload_ tab corresponds to the current _Upload_ popup in Sirius Web.

image::images/import-model.png[]
Copy link
Member

Choose a reason for hiding this comment

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

We can't realistically display all versions of everything that has been published on the server. We should probably list projects first and versions after that. This interface will have tens of thousands of entries if not more. A table with pagination is the bare minimum.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, maybe a paginated table with one row/model and a dropdown to select the version would be better. We'll rework the mockups for this feature.

@gdaniel gdaniel force-pushed the gda/doc/publication_shape branch from 78af8dd to b883f1d Compare December 20, 2024 17:20
@gdaniel
Copy link
Contributor Author

gdaniel commented Dec 20, 2024

@sbegaudeau I reworked the shape to integrate what we discussed in the comments. The shape now focuses on artifact publication, importing published artifacts will be in a different shape

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants