search

structurizr / java

Structurizr for Java
https://docs.structurizr.com/java
Apache License 2.0
1.02k stars 290 forks source link

Add support for various Ilograph features #332

Closed npouliquen closed 2 months ago

npouliquen commented 3 months ago

Description

Currently, the Ilograph exporter lacks support for some Ilograph features. Most notably to me:

  1. imports
  2. icons
  3. instanceOf

I believe these could be supported through the properties field in Workspace and Element. It seems like there is precedent for using properties to support features specific to an export type based on this line in the PlantUML exporter.

To support icon and instanceOf, there could simply be a check for those strings in the properties map in the writeElement method. Supporting imports is harder, as each Ilograph import has 2 properties, from and namespace. The way it needs to be formatted in the output is like this: 

imports: 
- from: ilograph/aws
  namespace: AWS

My best thought right now would be to just have a check for a property with key "imports" and a value of "- from: ilograph/aws\nnamespace: AWS". This looks kinda ugly, but I cannot think of a better way given the class structure of the structurizr library. Happy to hear if anyone has any better ideas. I'm more than willing to implement this feature myself, as it is something I would really value.

Priority

I'm willing to add this feature myself and raise a PR (please confirm approach first)

More information

No response

simonbrowndotje commented 3 months ago
  1. import

My initial though would be a comma-separated list of imports in a workspace property named ilograph.imports, in the form of something like: AWS:ilograph/aws

  1. icon

I've added support for icons, which will use a property named ilograph.icon on the element style if it exists, and the icon property on the element style otherwise.

  1. instanceOf

What would this be used for?

npouliquen commented 3 months ago
  1. I like your idea for import much more than mine. Using the colon as a token is a much more elegant solution. Great suggestion!
  2. I was not aware of the existing icon support. I couldn't find it when looking through the IlographExporter class, but I imagine it exists in a superclass.
  3. instanceOf in Ilograph serves as a way to reduce repetition in a diagram. It's a mechanism for inheritance. The example given in the Ilograph documentation is when diagramming a computer network, you could have 2 identical network switches deployed. These would have the same properties. Using instanceOf allows you to not repeat yourself redefining these properties. Here is a link to that explanation.
simonbrowndotje commented 3 months ago
  1. Done -> https://github.com/structurizr/java/commit/8dbdd7897bac3aa77c18f29cb9132fab49f4dcd6
  2. It wasn't an existing feature, I added it -> https://github.com/structurizr/java/commit/a6e531fefe72bc212439a7a5483af48d654af184
  3. Can you provide an example of how you'd use this?
npouliquen commented 3 months ago

For 3, I was diagramming a couple services my team owns, one upstream service, and one downstream. All the services are doing some async event handling. Each service has an SNS topic, SQS queue, and Lambda to process messages. I repeated this 4 times, which is fine, but having an abstract definition of an Ilograph resource which I can then say everything else is an instanceOf is a few less lines of writing overall.

Unrelated, but I appreciate you acting so quickly to implement these changes. A talk you gave on C4 came up in my YouTube recommended. I found the idea of models as code very compelling. I was playing around with it last week as a proof of concept. I am sold. Now I am going to pitch to my team a requirement to add C4 models with every commit that adds new functionality.

simonbrowndotje commented 3 months ago

You're welcome! Do you have a code example of how you want to use instanceOf? From the docs I can see how it's intended to be used, but I don't understand how this would work given the Ilograph code is being generated by the exporter. If you take the example from the docs:

Screenshot 2024-09-03 at 07 44 13

How would you create a relationship to/from an instance of the switch?

npouliquen commented 3 months ago

I typed out an explanation and only upon expanding on this idea did I realize it isn't useful, as the relationships would not be able to be defined in Structurizr DSL. The only way would be to also add support for the abstract property in Structurizr, but then you would also have to build in some inheritance structure in the library as well. I think that becomes quite complicated.

So, the only use of adding this would be to support importing resources from Ilograph's standard library. Currently, the only thing that's there is a theme for AWS resources. They claim this will expand soon, but given nothing has been added there in 3 years, I am doubtful. Given that there is already support for colors and now icons, the same theming can be done from Structurizr. I'm much less concerned about this specific feature now. I was initially intending to use it to do some abstraction in a separate file, which I would then import, but this won't work based on what I explained in the first paragraph.

simonbrowndotje commented 3 months ago

I typed out an explanation and only upon expanding on this idea did I realize it isn't useful, as the relationships would not be able to be defined in Structurizr DSL.

I came to the same conclusion, but I don't use Ilograph, so wasn't sure if I'd missed something. Thanks for the info!