Skip to content

[1861] Split responsibilities of SysONLibraryPublicationHandler #1874

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.

Sign up for GitHub

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

Jump to bottom
Open
flatombe wants to merge 1 commit into
base: cooldown
Choose a base branch
from
Open

[1861] Split responsibilities of SysONLibraryPublicationHandler #1874

flatombe wants to merge 1 commit into from
+305 −203

Conversation

@flatombe
Copy link
Contributor

@flatombe flatombe commented Jan 20, 2026 •
edited by AxelRICHARD
Loading

Bug: #1861

PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW

Auto review

  • Have you reviewed this PR? Please do a first quick review, It is very useful to detect typos and missing copyrights, check comments, check your code... The reviewer will thank you for that :)

Project management

  • Has the pull request been added to the relevant milestone?
  • 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:, type:)
  • Have the relevant issues been added to the same project milestone as the pull request?

Changelog and release notes

  • Has the CHANGELOG.adoc + doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc?
  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
  • In case of a change with a visual impact, are there any screenshots in the doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?
  • In case of a key change, has the change been added to Key highlights section in doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?

Documentation

  • Have you included an update of the documentation in your pull request? Please ask yourself if an update (installation manual, user manual, developer manual...) is needed and add one accordingly.

Tests

  • Is the code properly tested? Any pull request (fix, enhancement or new feature) should come with a test (or several). It could be unit tests, integration tests or cypress tests depending on the context. Only doc and releng pull request do not need for tests.

@AxelRICHARD AxelRICHARD added this to the 2026.3.0 milestone Jan 20, 2026
@AxelRICHARD AxelRICHARD linked an issue Jan 20, 2026 that may be closed by this pull request
Make SysON's library publication command independent from a specific input #1861
Open
@flatombe flatombe force-pushed the dev/FLA/refactor-publication branch from 35379e7 to 6a6350f Compare January 21, 2026 08:59
@flatombe flatombe changed the base branch from main to cooldown January 21, 2026 08:59
AxelRICHARD
AxelRICHARD reviewed Jan 21, 2026
View reviewed changes
...tion/src/main/java/org/eclipse/syson/application/publication/SysONSysMLLibraryPublisher.java
.toList();
}

protected Optional<SemanticData> publishAsLibrary(final ICause parentCause, final Collection<Resource> resources, final String libraryNamespace, final String libraryName,
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 21, 2026

Choose a reason for hiding this comment

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

Why some methods here are protected and some others private?

Copy link
Contributor Author

@flatombe flatombe Jan 21, 2026

Choose a reason for hiding this comment

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

For some reasons SysONLibraryPublicationHandler had a single private method but I missed it.
Fixed by setting the private ones to protected so downstream applications can easily fork the SysON default implementation to suit their own needs.

...tion/src/main/java/org/eclipse/syson/application/publication/SysONSysMLLibraryPublisher.java Outdated
.formatted(libraryMetadataAdapter.getNamespace(), libraryMetadataAdapter.getName(),
libraryMetadataAdapter.getVersion())));
});
// Ignore the resource if it isn't a library: all non-library resources are published in a single
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 21, 2026

Choose a reason for hiding this comment

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

Why this comment is here, not attached to anything?

Copy link
Contributor Author

@flatombe flatombe Jan 21, 2026

Choose a reason for hiding this comment

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

Fixed by rewording the comment and moving it before the call to getLibraryMetadata(Resource).ifPresent(...) which is the subject for that comment.

...tion/src/main/java/org/eclipse/syson/application/publication/SysONSysMLLibraryPublisher.java Outdated
result = new ErrorPayload(cause.id(),
List.of(new Message("Cannot publish SysML library from editing context '%s'.".formatted(libraryAuthoringEditingContext.getId()), MessageLevel.ERROR)));
} else if (this.librarySearchService.findByNamespaceAndNameAndVersion(libraryNamespace, libraryName, libraryVersion).isPresent()) {
// Sirius Web works assumes that published libraries are immutable.
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 21, 2026

Choose a reason for hiding this comment

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

works or assumes?

Copy link
Contributor Author

@flatombe flatombe Jan 21, 2026

Choose a reason for hiding this comment

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

A bit of both I guess :-) Fixed by rewording that comment.

...tion/src/main/java/org/eclipse/syson/application/publication/SysONSysMLLibraryPublisher.java

final Optional<SemanticData> maybePublishedLibrarySemanticData = this.publishAsLibrary(cause, resourcesToPublish, libraryNamespace, libraryName, libraryVersion, libraryDescription,
dependencies);
// After this transaction is done, SysONLibraryPublicationListener reacts by also creating the
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 21, 2026

Choose a reason for hiding this comment

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

Is this comments till true?

Copy link
Contributor Author

@flatombe flatombe Jan 21, 2026

Choose a reason for hiding this comment

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

Yes that is still the case.

...tion/src/main/java/org/eclipse/syson/application/publication/SysONSysMLLibraryPublisher.java Outdated
return result;
}

private IPayload doPublish(final ICause cause, final IEMFEditingContext emfEditingContext, final String libraryNamespace, final String libraryName,
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 21, 2026

Choose a reason for hiding this comment

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

Could you please describe in the javadoc what this method does?

Copy link
Contributor Author

@flatombe flatombe Jan 21, 2026

Choose a reason for hiding this comment

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

Fixed by adding Javadoc to that method.

@flatombe flatombe force-pushed the dev/FLA/refactor-publication branch from 6a6350f to 3b44578 Compare January 21, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AxelRICHARD AxelRICHARD AxelRICHARD left review comments

Assignees

No one assigned

Labels

pr: to review later priority: low

Projects

None yet

Milestone

2026.3.0

Development

Successfully merging this pull request may close these issues.

Make SysON's library publication command independent from a specific input

2 participants

@flatombe @AxelRICHARD