ZeligsoftDev / CX4CBDDS

CX4CBDDS component modeling and generation tool
Apache License 2.0
8 stars 5 forks source link

Attempting to modify anything in a model's referenced models does not work #368

Closed emammoser closed 2 years ago

emammoser commented 2 years ago

Issue and tracking information

If a user were to attempt to add a connector or change a port on a component in a referenced ICM from the model, nothing happens. When attempting to click Finish on the final dialog nothing happens. This should work. Users should be able to create connectors and other types in the referenced model from the model that is referencing them.

Additionally, we do have situations where we actually do not want this behavior (basically maintain the current behavior but with an error dialog if a user did try it). I am not 100% on how we would implement flagging a model as "read only" but we are open to suggestions. Obviously anything installed into the system would be read only. Some options that I could see is the ability direct papyrus or CX to some "file" specifying which models or actual resources in eclipse are read only. Maybe this is something that we work on in our plugins to force that read-only behavior.

Developer's time Estimated effort to fix (hours):

Developer's Actual time spent on fix (hours)

Issue reporter to provide a detailed description of the issue in the space below

eposse commented 2 years ago

Hi @j26151 . I've been assigned to work on this, but since I haven't worked on CX for a while, I was wondering if you could provide a minimal sample model? Thanks

emammoser commented 2 years ago

There isn't much to recreate here. I am attaching two models, BasicPubSub and ClientServer. I made ClientServer have a dynamic pathmap and I made BasicPubSub import it to reference it via. that pathmap and not a workspace relative path. If you open up just the BasicPubSub model and try to edit ClientServer, you will be unable to make any changes zeligsoft_368.zip .

eposse commented 2 years ago

Thanks. I'm having trouble getting the dev environment setup, so I'd like to confirm with you what you are using. If I recall, you are on CentOS 7, is that right? And you are using Java 11, Oracle or OpenJDK or something else?

J17359 commented 2 years ago

Thanks. I'm having trouble getting the dev environment setup, so I'd like to confirm with you what you are using. If I recall, you are on CentOS 7, is that right? And you are using Java 11, Oracle or OpenJDK or something else?

I imagine @ysroh could help you more than we can as he routinely is working bug fixes and new features for us. But yes, we're on CentOS 7.9 with the java-11-openjdk-11.0.14.1.1-1.el7_9.x86_64 RPM installed and set as default in our alternatives for java on the VM and explicitly set as the Java version in our Eclipse plugins.

eposse commented 2 years ago

Thanks. I was able to solve the issue with Paul's help.

I don't know if it affects you, but in case you use Xtend for your development, you will run into this. The problem is that the Papyrus 4.8 RCP includes a nightly build update site for Papyrus, so after installing Xtend or Xtext there will be a conflict between different versions of Google Guava and Google Guice. The solution is to remove the Papyrus nightly update site.

Anyway, regarding the issue, Young-Soo explained it to me, so I'm going to finish setting up and start to investigate it properly.

eposse commented 2 years ago

Ok, I've spent the last week or so investigating this and attempting to implement it, but I've found that there is no simple way of making referenced models with a dynamic pathmap editable in the model explorer. It looks like most of Papyrus is designed to use platform resource URIs and pathmaps are meant only for (read-only) libraries. Hence, it looks like a considerable amount of work would be needed to override large parts of Papyrus to support this, at least directly.

There could be some alternatives. For example, instead of being able to directly edit the referenced model in the Model Explorer, we could add a context menu option to open the referenced model in order to edit it. That would enable you to get to the model without having to manually look for it in the Project Explorer. Would this be an acceptable alternative?

As to making models read-only, maybe we could add a flag on a model to mark it as read-only, modifying the meta-model itself so that you get the flag in the model's property view, although of course a user could just clear the flag. Or a text file listing read-only models, as you suggest. These options could be used when attempting to edit a model and either deny any changes or prompt the user to confirm if they want to make changes.

emammoser commented 2 years ago

From reading the comment above, I gather that the limitation is that ".. there is no simple way of making referenced models with a dynamic pathmap editable in the model explorer"

Is this a limitation of just dynamic pathmaps? If we were including these model with workspace relative paths would everything work fine? Dynamic pathmaps as accessible by the user and not just through installed model libraries was a new feature that we had added to Papyrus, so I would definitely want to explore this more if it really is just that dynamic pathmaps are limiting us here.

ysroh commented 2 years ago

Ernesto is going to try one more thing before we conclude that this is not possible. I suspect that it might not be possible due to the limitation of pathmap in general, not just the dynamic pathmap. If you are referencing another model by platform URI (referencing external model without using Dynamic Pathmap via "search in the same project" option in the selection dialog), then it is possible to make the referenced model editable by right click and selecting "Enable write" menu. However, Papyrus does not support the editing of the path-mapped resource by default since the pathmap is designed to reference installed model libraries and we are manipulating the pathmap during runtime in order to dynamically locate the libraries that are located in the workspace. I think it would make more sense to open the original model to edit ICM models instead of trying to edit them in the referenced models. We will try one more thing and let you know if it would be even possible to do it or not. Then we can discuss more options.

ghost commented 2 years ago

This is fundamentally a workflow issue. It is natural and easy for users to make changes to a model or models by editing component instances, ports, interconnects, port types, and properties on selected elements seen visually on a composite structure diagram that is open in the main drawing window. All using the Palette, popup menus and properties pane, while the drawing remains visible for context. Ideally this should be doable without regard to which models or projects the visual elements the user is interacting with are actually stored within. Adding or removing ports from components, typing ports, or navigating to the underlying message or interface that a port is typed by, are all most easily done visually from a selected element on a drawing as the unifying reference for all graphically associated elements. But if you try to edit something referenced on a diagram and the user interface doesn't allow it (nothing happens??), natural workflow is broken. You have to abandon the drawing and start navigating through project and model explorer views to find the thing on the diagram that you want to change, and figure out why nothing is happening. And if you change the model being referenced to hunt down which one a port or message type is actually stored within, the diagram goes away and the user loses all context of what they were trying to do. It gets very confusing. Given this, maybe there are other approaches to helping solve this workflow problem when modeling project elements happen to be stored in separate models, including models in separate path-mapped projects.

ysroh commented 2 years ago

Thanks for the clarification on the workflow. We know exactly what the workflow is, so we will try to develop a solution for this and let you know.

ysroh commented 2 years ago

I have a question regarding the workflow. Do you expect to edit the referenced dynamic pathmap model from multiple editors?

e.g.,) Case 1. Model A and B both reference path mapped model P. User opens model A and B at the same time and make modifications to the path mapped model P indirectly. Case 2. Model A references path mapped model P. User opens the model A and P at the same time and make modifications to the model P.

In both cases, you are editing model P directly or indirectly from different editors, and Papyrus does not handle the above scenarios well. You may need to close and reopen the editors if you encounter problems.

Do you think you can live with the above limitations? The workaround will be to work on a single model at a time if you intend to modify the same referenced model.

I will commit the changes once reviewed, and you can try the feature and let me know.

ysroh commented 2 years ago

You can control the read-only mode from the model properties view. If you configure the path mapped model to be "Model Library" then the model will not be editable from the referenced models. image

ghost commented 2 years ago

Based upon the two use cases described, your limited solution does not sound like a problem. In general the workflow described only requires editing the referenced model indirectly from one parent model & structure diagram that leverages element types defined within it. While use case 1 seems quite unlikely, use case 2 could possibly happen inadvertently once editors for both the referencing and referenced models had been opened due to palette or pop-up menu indirect use from a diagram. For instance, the user might decide at a later point, without closing either of the 2 open models, to leave the diagram view workflow and go directly to the project/model explorer view of the referenced model, in order to edit the fields in a data structure or message/interface type (vs. a component port or connection on a diagram). It is not uncommon to switch workflow to using these views vs. diagram views for detailed content. IF a user were to do this inadvertently, what would happen? Something bad like model corruption? Nothing? Get a warning to close the referencing model? Something else?

ysroh commented 2 years ago

You can use the model explorer context menu from the referencing model to create element for the referenced model.

What will happen is that the editor will throw some exceptions and you will not be able to access the model explorer view. No model corruptions. You just need to reopen editors and everything will be fine.

ysroh commented 2 years ago

In order words, you can edit the fields in a data structure or message/interface type directly from the referencing model without opening the referenced model.

image

ghost commented 2 years ago

Great, all sounds good & worth trying out.

ysroh commented 2 years ago

Fix merged to the "papyrus" branch.

emammoser commented 2 years ago

This looks really good on my end. I did see one issue that we would like fixed. If Model A references Model B, and Model B is marked as a Model Library (read only), and I attempt to make a change in Model A to something in Model B (add a port to a component in model B from an assembly in Model A), nothing happens. We would like to see an error dialog pop up to just tell the user "You can't make this change because you are attempting to modify which is a read-only model library"

I have an additional question or feature request as well. Would it be possible to make toggling a model as a model library simpler in the UML itself? Our use-case for a model library is that we would expect to have a Model A not be marked as a model library ever. However, when we package this model library up as an RPM to install on other machines, we would like the ability via command line or a SED script to toggle it as model library such that when it is installed it would be marked as read-only. Currently to do this it appears that we would need to :

  1. find the end of the XML file, create a new "" tag
  2. Make up a random UUID for the xmi:id
  3. Set a base package attribute to the ID of the model itself

This could probably be done fairly easily with a python script but it would be easier to have the "" tag always present with an attribute of "read-only" or "editable" set to true or false. Thoughts?

ysroh commented 2 years ago

See comments inline

This looks really good on my end. I did see one issue that we would like fixed. If Model A references Model B, and Model B is marked as a Model Library (read only), and I attempt to make a change in Model A to something in Model B (add a port to a component in model B from an assembly in Model A), nothing happens. We would like to see an error dialog pop up to just tell the user "You can't make this change because you are attempting to modify which is a read-only model library"

ys] Will work on it.

I have an additional question or feature request as well. Would it be possible to make toggling a model as a model library simpler in the UML itself? Our use-case for a model library is that we would expect to have a Model A not be marked as a model library ever. However, when we package this model library up as an RPM to install on other machines, we would like the ability via command line or a SED script to toggle it as model library such that when it is installed it would be marked as read-only. Currently to do this it appears that we would need to :

  1. find the end of the XML file, create a new "standard:ModelLibrary" tag
  2. Make up a random UUID for the xmi:id
  3. Set a base package attribute to the ID of the model itself

This could probably be done fairly easily with a python script but it would be easier to have the "standard:ModelLibrary" tag always present with an attribute of "read-only" or "editable" set to true or false. Thoughts?

ys] One way to address this issue is to call a function in CX to add ModelLibrary stereotype. However, this requires running the tool in headless mode to invoke the function and may be too heavy for a simple thing like this. So I would suggest an alternative way to handle this. I can update the code to use the annotation instead of the model library profile. This way we can add the annotation with the default value set to "false" and it will be easy to change the value to "true" when required. The only downside of this solution is that all ICM models require model migration to insert the annotation with the default value.

I will make changes and let you know when they are ready.

emammoser commented 2 years ago

Sounds good, thank you.

ysroh commented 2 years ago

All required changes, including the migration, are merged into the papyrus branch. You can change the "model_library" value of the "zcx" annotation to "true" to make the pathmapped model read-only. image

Let me know if you find any issues with this commit.

emammoser commented 2 years ago

This all looked good to me, thank you.

ysroh commented 2 years ago

Closing...