Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

#47 Add new C4PlantUmlWriter with C4-PlantUML support#52

Merged
simonbrowndotje merged 6 commits intostructurizr:masterfrom
kirchsth:master
Nov 19, 2019
Merged

#47 Add new C4PlantUmlWriter with C4-PlantUML support#52
simonbrowndotje merged 6 commits intostructurizr:masterfrom
kirchsth:master

Conversation

@kirchsth
Copy link
Copy Markdown

  • Add new C4PlantUmlWriter with C4-PlantUML support (issue Are there any plans to extend PlantUMLWriter with support for C4-PlantUML? #47)
  • ModelItem returns a list of all tags via GetAllTag()
  • RelationshipView support an additional list of tags which can be used in the diagram via ViewTags.
    RelationshipView.GetAllTag() returns a merged list of ViewTags and all Relationship tags
  • Update relationship descriptions and technology in BigBankPlc example
  • Update PlantUMLWriterTests and C4PlantUmlWriterTests that the expected text can be (simpler) reused as UML definition

- Add new C4PlantUmlWriter with C4-PlantUML support (issue structurizr#47)
- ModelItem returns a list of all tags via GetAllTag()
- RelationshipView support an additional list of tags which can be used in the diagram via ViewTags.
  RelationshipView.GetAllTag() returns a merged list of ViewTags and all Relationship tags
- Update relationship descriptions and technology in BigBankPlc example
- Update PlantUMLWriterTests and C4PlantUmlWriterTests that the expected text can be (simpler) reused as UML definition
Copy link
Copy Markdown
Contributor

@simonbrowndotje simonbrowndotje left a comment

Choose a reason for hiding this comment

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

Hi, thanks very much for adding this. It all looks good except for the addition of the "view tags" property on the RelationshipView class. Having a separate list of tags for relationship views is going to potentially confuse people, as the Structurizr service doesn't support this concept. Is there another way to do this without changing the RelationshipView class?

@kirchsth
Copy link
Copy Markdown
Author

Hi Simon, the "view tags" are a view specific extension like x and y in "ElementView". In my case I use it and add directions like up/down/left/right but it could be generic enough to add other view related information too. And I (re-)used "tags" that I could even defined a common layout directly in the model and reuse it in all views (like person is on top of view).
What would be your suggestion that I have something like "x,y". I think style would be wrong.
Helmut

@simonbrowndotje
Copy link
Copy Markdown
Contributor

Thanks, I understand. This is primarily a client library for the Structurizr service though, and I don't really want any changes to the core classes - having tags on the RelationshipView class is going to potentially confuse people who are using the Structurizr service, which doesn't support this concept.

@simonbrowndotje
Copy link
Copy Markdown
Contributor

I've added a properties property to the ElementView and RelationshipView classes ... could you use that instead?

@kirchsth
Copy link
Copy Markdown
Author

kirchsth commented Nov 2, 2019

Hi Simon,
1. properties are fine I will update my implementation (remove all tags; and add to the Relationship Property["PUML_DefaultLayout"]; and to the RelationshipView property ["PUML_Layout"].
2. during my C4-PlantUML pull request, details see plantuml-stdlib/C4-PlantUML#34 it starts a discussion about "Container" or "ContainerInstance", In your current model you store only health check urls and no "productive" url/endpoint.
Can you suggest a property which should be displayed?
Thank you
Hemut

@simonbrowndotje
Copy link
Copy Markdown
Contributor

Thanks. I'm not sure what you mean by, "In your current model you store only health check urls and no "productive" url/endpoint. Can you suggest a property which should be displayed?" ... could you elaborate?

@kirchsth
Copy link
Copy Markdown
Author

kirchsth commented Nov 4, 2019

In my branch https://github.com/kirchsth/C4-PlantUML I have a hypothetical sample with tenants, and both are children of the same node (and in your case both would have a different health check url).
(I found a bug in my sample: both should have the same description "Stores Tenant related information")
I know database is not a good health check sample but I think it explains the idea.

@simonbrowndotje
Copy link
Copy Markdown
Contributor

Yes, I think you're correct on both counts...

  • Each database should be labelled "Stores Tenant related information".
  • Each database instance would have a different health check URL. I wouldn't necessarily add this to the diagram though, I certainly don't with Structurizr.

@kirchsth
Copy link
Copy Markdown
Author

kirchsth commented Nov 4, 2019

Following your current pattern with the FailoverTag, then your solution could look like:
a) the diagram would display one "Tenant" OK and the other "Tenant" NotOK
b) and each tenant has it own Tag (instead of "FailoverTag", "Tenant1Tag" and "Tenant2Tag")
c) and the diagram uses e.g. 2 different (style based) colors
d) and the user knows which "Tenant" is failing
Is my assumption correct?

@simonbrowndotje
Copy link
Copy Markdown
Contributor

It's really up to you, and it can be visualised however you like, there are no "rules" here. For the Structurizr service, the colour of the container instance is changed based upon the health check passing (e.g. https://structurizr.com/share/38000/health) ... but I don't think many people use this feature to be honest. It's also worth noting that this is a dynamic diagram, not a static image.

@kirchsth
Copy link
Copy Markdown
Author

kirchsth commented Nov 4, 2019

I agree, it is a static diagram. I used the "different health check urls" only to demonstrate that there could be different instances in one node and a user cannot distinguish between the 2 instances (except the different displayed relationships which have different descriptions)

KIRCHSTH added 4 commits November 9, 2019 14:20
- RelationshipView.GetAllTag() support removed
- C4PlantUmlWriter supports Relationship and RelationshipView.Property[Direction] (can be set via .SetDirection())
- C4PlantUmlWriter supports Component and Container.Property[IsDatabase] (can be set via .SetIsDatabase())
- Node and Container in deployment diagrams
- Interact2 in dynamic diagrams (Interact are relations with the automatically increased index; Interact2 uses Interact layout with explicit order)
…file error MSB5010: No file format header found)
@kirchsth
Copy link
Copy Markdown
Author

Hi Simon,
all my changes are in. Can you please review it? I hope you like it.
Best regards
Helmut

@kirchsth
Copy link
Copy Markdown
Author

@simonbrowndotje: do I have to make something special that state "Changes requested" is changed?

@simonbrowndotje
Copy link
Copy Markdown
Contributor

Thanks very much ... I'm travelling at the moment, but this is on my todo list for next week when I get back home.

@simonbrowndotje simonbrowndotje merged commit 3761cd9 into structurizr:master Nov 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants