archimatetool / archi-scripting-plugin

jArchi - Scripting for Archi: ArchiMate Modelling Tool
https://www.archimatetool.com
118 stars 33 forks source link

Support for Specializations #97

Closed Phillipus closed 2 years ago

Phillipus commented 2 years ago

See https://github.com/archimatetool/archi/wiki/%22ArchiMate-language-customization%22-Project

jArchi support for specializations and other new features are now in the dev branch

We can discuss jArchi speciific development of specializations here...

Phillipus commented 2 years ago

I've implemented all of the jArchi features as defined in the above link.


var selected = selection.first();
var concept = selected.concept;

// Set a specialization on a concept.
// If the specialization doesn't exist in the model it will be created
concept.specialization = "My Specialization";

// Create a new specialization in the model with no image set
var spec1 = model.createSpecialization("Oscar", "business-actor", null);

// Add an image from file to the model
// This returns the internal image file name which can be used as a reference to the image
// (if images are not used, they will not be saved in the *.archimate file)
var image = model.createImage("image.png");

// Create a new specialization with that image
var spec2 = model.createSpecialization("Oscar 2", "business-actor", image);

// Set the concept to this specialization
// Note we use the name of the concept so both of these work
concept.specialization = "Oscar 2";
//concept.specialization = spec2.name;

// Set icon show
selected.showIcon = SHOW_ICON.NEVER;

// Set image source
selected.imageSource = IMAGE_SOURCE.CUSTOM;

// Set image position
selected.imagePosition = IMAGE_POSITION.FILL;

// Set image
var image2 = model.createImage("image2.png");
selected.image = image2;

// Delete all of them
model.specializations.forEach(spec => {
    spec.delete();
});
Phillipus commented 2 years ago

I didn't implement model.createImage(data) because I thought most users would have an image in a file. But we can implement it if needed (what format?)

Things missing that would be useful:

specialization.image = newImage;
specialization.name= newName;
specialization.type= newType;  // Only if not in use
jbsarrodie commented 2 years ago

HI. I'll test all this (and CSV) next week).

Things missing that would be useful

You're right, these could be helpful.

I didn't implement model.createImage(data) because I thought most users would have an image in a file. But we can implement it if needed (what format?)

Agree. I'll have to think about it (maybe for another release).

// Add an image from file to the model
// This returns the internal image file name which can be used as a reference to the image
// (if images are not used, they will not be saved in the *.archimate file)
var image = model.createImage("image.png");

Does this checks if an image already exists with the same content ?

Phillipus commented 2 years ago

Does this checks if an image already exists with the same content ?

Yes. It's a wrapper around ArchiveManager.

Another thing might be useful:

// Get a specialization by name and type
var specialization = model.findSpecialization(name, type);
// So that we can do stuff
specialization.image = newImage;
concept.specialization = specialization.name;
Phillipus commented 2 years ago

Elements and relationships should have a new String attribute named specialization. Reading this attribute returns the specialization name (or null if none defined),

The problem with that is that I want to access the specialization object from the concept so I can get and set its attributes:

var oldname = concept.specialization.name;
concept.specialization.name = newName;

Is there a reason to return the specialization name rather than the specialization object?

Otherwise we have to do something like:

var specialization = model.findSpecialization(concept.specialization, concept.type);
specialization.name = newName;
jbsarrodie commented 2 years ago

Is there a reason to return the specialization name rather than the specialization object?

Yes, a specialization is fully defined by its name (because the associated type can inferred from the concept it is applied to. Almost like native ArchiMate types.

And I think most use-cases will be to simply get the name of the specialization set on an object or set a specialization already knowing its name. While getting the specialization configuration from a concept to change its definition should be very rare (I see no use-case for that at the moment).

So IMHO, no need to do something like:

var specialization = model.findSpecialization("My Specialization", concept.type);
concept.specialization = specialization;

While this is enough:

concept.specialization = "My Specialization";

And for your use-case, we can still define an alternate signature for model.findSpecialization() which takes a concept as an input and return the specialization object.

Phillipus commented 2 years ago

The use case is to support what one can do in the Specializations manager:

So jArchi needs to return a specialization object. So perhaps model.findSpecialization(name, type) is the most generic.

jbsarrodie commented 2 years ago

I think we have to different use cases:

Do what can be done in the Specialization manager

This involved:

Get/Set specialization on concepts

This should be as easy as getting/setting a type, thus my suggestion to use a string and not a specialization object because in most (if not all) cases, user will want to get the name of the specialization or set a specialization whose name is known.

jbsarrodie commented 2 years ago

Hi, I've finally found time to test. That's really good and covers all needs, but I have some remarks.

Specialization should accessible from visual object

I think that we should expose specialization also from visual object, this would be in line with name, doc, type and properties that are already accessible.

Don't create specialization on the fly

// Set a specialization on a concept. // If the specialization doesn't exist in the model it will be created

I don't think we should create specialization on the fly because this could make it difficult to spot errors on code. IMHO this should work like type: raise an error if the specialization doesn't exist.

Here's an example of bad code that works while it shouldn't:

var el = // set el to an element of any type but business actor
model.createSpecialization("Oscar", "business-actor", null);
el.specialization = 'Oscar';

In this example, Oscar has been defined as a specialization of business actor so can't be applied, but in this case another specialization with the same name but for a different type will be created on the fly, making it difficult to spot the error if we loop on a collection but forgot to filter it for business actors.

Should model.createSpecialization() return existing specialization?

Create a new specialization with the given name, concept type and image. If a specialization of the same name and type already exists, that is returned.

I'm not so sure of this. That can be useful, but current implementation doesn't check the image, so I can get a specialization which isn't exactly the one I specified in the call. Maybe we should raise an error in such case (ie. if a specialization with same name and type exists but with a different image).

But I also wonder if we should do it at all. After all, if want to create something and it alreasy exists, that's an issue of some sort that I should be aware of, no?

Thinking out loud... My issue is maybe that the create keyword is used with two different semantics in our API: it only creates for concepts, view, folder... but it creates or returns an already existing object for images and specialization. This might be confusing for people.

Regretion in exception handling?

I've noticed that when an exception is raised, the full stack trace gets printed on the jArchi's console. Is that a regression?

Potential enhancement for images

I would be nice to be able to access image's dimensions (image.width and images.height). This would allow some nice use-cases involving rendering a view as an image file, loading it as an image and set it on a note with "fill" mode. The last step would be to resize the note to the acyual size of the initial view. Such thing would allow to keep some "imutable" copy of a view and to add other elements on top of it.

Phillipus commented 2 years ago

I've noticed that when an exception is raised, the full stack trace gets printed on the jArchi's console. Is that a regression?

Deliberate. It prints the first 12 lines of the stack trace. Why? Because a user came across a bug where the output was simply Script Error: java.lang.NullPointerException and that was not very helpful.

Phillipus commented 2 years ago

Deliberate. It prints the first 12 lines of the stack trace. Why? Because a user came across a bug where the output was simply Script Error: java.lang.NullPointerException and that was not very helpful.

Because runtime errors are re-directed to the Console we want to know if it's an internal error like a NPE so the stack trace is helpful. On the other hand, if it's an error in the script we don't want to see the stack trace. I'll try to differentiate...

Phillipus commented 2 years ago

In this example, Oscar has been defined as a specialization of business actor so can't be applied, but in this case another specialization with the same name but for a different type will be created on the fly, making it difficult to spot the error if we loop on a collection but forgot to filter it for business actors.

OK, I understand your point but what if the user does want to set and create a Specialization called Oscar for a Business Role as well as a Business Actor? Specializations of the same name (but different type) are allowed. I guess the user must create a Specialization explicitly for that type first?

var el = // set el to business-role
model.createSpecialization("Oscar", "business-actor", null);
model.createSpecialization("Oscar", "business-role", null);
el.specialization = 'Oscar';
Phillipus commented 2 years ago

I would be nice to be able to access image's dimensions (image.width and images.height).

At the moment the Image object is a String for the Image Path. We would need to make it a JS object.

Phillipus commented 2 years ago

I would be nice to be able to access image's dimensions (image.width and images.height).

Done.

Depends on the latest commit in master of the core Archi code so we can get the ImageData from the ArchiveManager.

Phillipus commented 2 years ago

At the moment the Image object is a String for the Image Path. We would need to make it a JS object.

Also done. Image is a JS object with fields path, width and height.

Phillipus commented 2 years ago

BTW - wiki for new stuffs is here:

https://github.com/archimatetool/archi-scripting-plugin/wiki/Features-In-Development

jbsarrodie commented 2 years ago

OK, I understand your point but what if the user does want to set and create a Specialization called Oscar for a Business Role as well as a Business Actor? Specializations of the same name (but different type) are allowed. I guess the user must create a Specialization explicitly for that type first?

Yes, that's how I see it (IMHO, too much magic can hurt sometime)

Also done. Image is a JS object with fields path, width and height.

You're a magician ;-)

BTW - wiki for new stuffs is here:

I've seen it, that looks great.

So I only have to test those new things around image and we're done

Phillipus commented 2 years ago

So I only have to test those new things around image and we're done

More stuffs incoming...(tomorrow)

Phillipus commented 2 years ago

Specialization should accessible from visual object

Done.

Phillipus commented 2 years ago

Don't create specialization on the fly el.specialization = 'Oscar';

An exception is now thrown if "Oscar" doesn't exist for that concept type - create it first you lazy thing! ;-)

Should model.createSpecialization() return existing specialization?

Nope. An exception is now thrown if you try to create a duplicate Specialization.

jbsarrodie commented 2 years ago

Everything is working as expected. I've just added support for model.createSpecialization(name, conceptType) as a quick way to create a new specialization without image (more in JS spirit than using 'null').

I think we can now close this issue :-)

Phillipus commented 2 years ago

I've just added support for model.createSpecialization(name, conceptType)

Thanks. I thought I'd done that one, as the wiki has it documented.

I think we can now close this issue :-)

OK. I'm going to squash and rebase the dev branch onto master.

jbsarrodie commented 2 years ago

I thought I'd done that one, as the wiki has it documented.

I updated the wiki after the commit ;-)

Phillipus commented 2 years ago

I updated the wiki after the commit ;-)

D'oh!