diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5db3bec3..eb193a5e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,8 @@ History Next Release ------------ +* Fix: Don't duplicate relationships if ``add_nearest_neighbours()`` called twice (#63) +* Fix: Support blank diagrams descriptions from the Structurizr UI (#40) 0.3.0 (2020-11-29) ------------------ diff --git a/examples/big_bank.py b/examples/big_bank.py index 1d222313..2ab97eb3 100644 --- a/examples/big_bank.py +++ b/examples/big_bank.py @@ -152,14 +152,16 @@ def create_big_bank_workspace(): single_page_application.tags.add(web_browser_tag) mobile_app = internet_banking_system.add_container( "Mobile App", - "Provides a limited subset of the Internet banking functionality to customers via their mobile device.", + "Provides a limited subset of the Internet banking functionality to " + "customers via their mobile device.", "Xamarin", id="mobileApp", ) mobile_app.tags.add(mobile_app_tag) web_application = internet_banking_system.add_container( "Web Application", - "Delivers the static content and the Internet banking single page application.", + "Delivers the static content and the Internet banking single page " + "application.", "Java and Spring MVC", id="webApplication", ) @@ -171,7 +173,8 @@ def create_big_bank_workspace(): ) database = internet_banking_system.add_container( "Database", - "Stores user registration information, hashed authentication credentials, access logs, etc.", + "Stores user registration information, hashed authentication credentials, " + "access logs, etc.", "Relational Database Schema", id="database", ) @@ -188,8 +191,9 @@ def create_big_bank_workspace(): api_application.uses(email_system, "Sends e-mail using", technology="SMTP") # components - # - for a real-world software system, you would probably want to extract the components using - # - static analysis/reflection rather than manually specifying them all + # - for a real-world software system, you would probably want to extract the + # components using static analysis/reflection rather than manually specifying + # them all signin_controller = api_application.add_component( name="Sign In Controller", @@ -211,7 +215,8 @@ def create_big_bank_workspace(): ) security_component = api_application.add_component( name="Security Component", - description="Provides functionality related to signing in, changing passwords, etc.", + description="Provides functionality related to signing in, changing passwords, " + "etc.", technology="Spring Bean", id="securityComponent", ) @@ -353,7 +358,8 @@ def create_big_bank_workspace(): secondary_database_server.tags.add(failover_tag) secondary_database = secondary_database_server.add_container(database) - # # model.Relationships.Where(r=>r.Destination.Equals(secondary_database)).ToList().ForEach(r=>r.tags.add(failover_tag)) + # model.Relationships.Where(r=>r.Destination.Equals(secondary_database)).ToList() + # .ForEach(r=>r.tags.add(failover_tag)) data_replication_relationship = primary_database_server.uses( secondary_database_server, "Replicates data to" ) @@ -400,7 +406,8 @@ def create_big_bank_workspace(): component_view.add(email_system) component_view.paper_size = PaperSize.A5_Landscape - # systemLandscapeView.AddAnimation(internet_banking_system, customer, mainframe_banking_system, emailSystem) + # systemLandscapeView.AddAnimation(internet_banking_system, customer, + # mainframe_banking_system, emailSystem) # systemLandscapeView.AddAnimation(atm) # systemLandscapeView.AddAnimation(customerServiceStaff, back_office_staff) @@ -418,20 +425,24 @@ def create_big_bank_workspace(): # componentView.AddAnimation(singlePageApplication, mobile_app) # componentView.AddAnimation(signinController, securityComponent, database) - # componentView.AddAnimation(accountsSummaryController, mainframe_banking_systemFacade, mainframe_banking_system) + # componentView.AddAnimation(accountsSummaryController, + # mainframe_banking_systemFacade, mainframe_banking_system) # componentView.AddAnimation(resetPasswordController, emailComponent, database) # # dynamic diagrams and deployment diagrams are not available with the Free Plan - # DynamicView dynamicView = views.CreateDynamicView(apiApplication, "SignIn", "Summarises how the sign in feature works in the single-page application.") + # DynamicView dynamicView = views.CreateDynamicView(apiApplication, "SignIn", + # "Summarises how the sign in feature works in the single-page application.") # dynamicView.Add(singlePageApplication, "Submits credentials to", signinController) # dynamicView.Add(signinController, "Calls isAuthenticated() on", securityComponent) - # dynamicView.Add(securityComponent, "select * from users where username = ?", database) + # dynamicView.Add(securityComponent, "select * from users where username = ?", + # database) # dynamicView.PaperSize = PaperSize.A5_Landscape development_deployment_view = views.create_deployment_view( software_system=internet_banking_system, key="DevelopmentDeployment", - description="An example development deployment scenario for the Internet Banking System.", + description="An example development deployment scenario for the Internet " + "Banking System.", environment="Development", ) development_deployment_view.add(developer_laptop) @@ -440,7 +451,8 @@ def create_big_bank_workspace(): live_deployment_view = views.create_deployment_view( software_system=internet_banking_system, key="LiveDeployment", - description="An example live deployment scenario for the Internet Banking System.", + description="An example live deployment scenario for the Internet Banking " + "System.", environment="Live", ) live_deployment_view += big_bank_data_center diff --git a/examples/financial_risk_system.py b/examples/financial_risk_system.py index 37dd2650..7e920ca7 100644 --- a/examples/financial_risk_system.py +++ b/examples/financial_risk_system.py @@ -127,7 +127,10 @@ def main() -> Workspace: contextView = views.create_system_context_view( software_system=financial_risk_system, key="Context", - description="An example System Context diagram for the Financial Risk System architecture kata.", + description=( + "An example System Context diagram for the Financial Risk System " + "architecture kata." + ), ) contextView.add_all_software_systems() contextView.add_all_people() diff --git a/src/structurizr/view/static_view.py b/src/structurizr/view/static_view.py index f60cf48e..14bba512 100644 --- a/src/structurizr/view/static_view.py +++ b/src/structurizr/view/static_view.py @@ -72,7 +72,7 @@ def hydrate_arguments(cls, static_view_io: StaticViewIO) -> Dict: @abstractmethod def add_all_elements(self) -> None: """Add all permitted elements from a model to this view.""" - pass + pass # pragma: no cover def add( self, @@ -106,7 +106,7 @@ def add_nearest_neighbours( element_type: Union[Type[Person], Type[SoftwareSystem], Type[Container]], ) -> None: """Add all permitted elements from a model to this view.""" - self._add_element(element, True) + self._add_element(element, False) # TODO(ilaif): @midnighter - Should we move to @property instead # of get_X()? More pythonic. diff --git a/src/structurizr/view/view.py b/src/structurizr/view/view.py index 6bd81529..205b18b4 100644 --- a/src/structurizr/view/view.py +++ b/src/structurizr/view/view.py @@ -42,7 +42,7 @@ class ViewIO(BaseModel, ABC): """ key: str - description: str + description: str = "" software_system_id: Optional[str] = Field(default=None, alias="softwareSystemId") paper_size: Optional[PaperSize] = Field(default=None, alias="paperSize") automatic_layout: Optional[AutomaticLayoutIO] = Field( @@ -173,14 +173,23 @@ def _add_relationship(self, relationship: Relationship) -> RelationshipView: """Add a single relationship to this view. Returns: - The new view if both the source and destination for the relationship are - in this view, else `None`. + The new (or existing) view if both the source and destination for the + relationship are in this view, else `None`. """ if self.is_element_in_view(relationship.source) and self.is_element_in_view( relationship.destination ): - view = RelationshipView(relationship=relationship) - self.relationship_views.add(view) + view = next( + ( + rv + for rv in self.relationship_views + if rv.relationship is relationship + ), + None, + ) + if not view: + view = RelationshipView(relationship=relationship) + self.relationship_views.add(view) return view return None diff --git a/tests/unit/view/test_static_view.py b/tests/unit/view/test_static_view.py new file mode 100644 index 00000000..d7eb9a30 --- /dev/null +++ b/tests/unit/view/test_static_view.py @@ -0,0 +1,72 @@ +# Copyright (c) 2020 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +"""Ensure the expected behaviour of StaticView.""" + + +from structurizr.model import Model, Person, SoftwareSystem +from structurizr.view.static_view import StaticView + + +class DerivedView(StaticView): + """Mock class for testing.""" + + def add_all_elements(self) -> None: + """Stub method because base is abstract.""" + pass + + +def test_add_nearest_neighbours(): + """Test basic behaviour of add_nearest_neighbours.""" + model = Model() + sys1 = model.add_software_system(name="System 1") + sys2 = model.add_software_system(name="System 2") + person = model.add_person(name="Person 1") + sys1.uses(sys2) + person.uses(sys1) + + # Check neighbours from outbound relationships + view = DerivedView(software_system=sys1, description="") + view.add_nearest_neighbours(sys1, SoftwareSystem) + assert any((elt_view.element is sys1 for elt_view in view.element_views)) + assert any((elt_view.element is sys2 for elt_view in view.element_views)) + assert not any((elt_view.element is person for elt_view in view.element_views)) + assert len(view.relationship_views) == 1 + + # Check neighbours from inbound relationships + view = DerivedView(software_system=sys1, description="") + view.add_nearest_neighbours(sys2, SoftwareSystem) + assert any((elt_view.element is sys1 for elt_view in view.element_views)) + assert any((elt_view.element is sys2 for elt_view in view.element_views)) + assert not any((elt_view.element is person for elt_view in view.element_views)) + assert len(view.relationship_views) == 1 + + +def test_add_nearest_neighbours_doesnt_dupe_relationships(): + """Test relationships aren't duplicated if neighbours added more than once. + + See https://github.com/Midnighter/structurizr-python/issues/63. + """ + model = Model() + sys1 = model.add_software_system(name="System 1") + sys2 = model.add_software_system(name="System 2") + sys1.uses(sys2) + view = DerivedView(software_system=sys1, description="") + view.add_nearest_neighbours(sys1, SoftwareSystem) + assert len(view.relationship_views) == 1 + + # The next line should not add any new relationships + view.add_nearest_neighbours(sys1, Person) + assert len(view.relationship_views) == 1 diff --git a/tests/unit/view/test_view.py b/tests/unit/view/test_view.py new file mode 100644 index 00000000..1d47a9fd --- /dev/null +++ b/tests/unit/view/test_view.py @@ -0,0 +1,99 @@ +# Copyright (c) 2020 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +"""Ensure the expected behaviour of View.""" + +from structurizr.model import Model +from structurizr.view.view import View, ViewIO + + +class DerivedView(View): + """Mock class for testing.""" + + pass + + +def test_add_relationship_doesnt_duplicate(): + """Test that adding a relationships twice doesn't duplicate it.""" + model = Model() + sys1 = model.add_software_system(name="System 1") + sys2 = model.add_software_system(name="System 2") + rel = sys1.uses(sys2) + + view = DerivedView(software_system=sys1, description="") + view._add_element(sys1, False) + view._add_element(sys2, False) + + rel_view1 = view._add_relationship(rel) + assert len(view.relationship_views) == 1 + rel_view2 = view._add_relationship(rel) + assert len(view.relationship_views) == 1 + assert rel_view2 is rel_view1 + + +def test_add_relationship_for_element_not_in_view(): + """Ensures relationships for elements outside the view are ignored.""" + model = Model() + sys1 = model.add_software_system(name="System 1") + sys2 = model.add_software_system(name="System 2") + rel = sys1.uses(sys2) + + view = DerivedView(software_system=sys1, description="") + view._add_element(sys1, False) + + # This relationship should be ignored as sys2 isn't in the view + rel_view1 = view._add_relationship(rel) + assert rel_view1 is None + assert view.relationship_views == set() + + +def test_adding_all_relationships(): + """Test adding all relationships for elements in the view.""" + model = Model() + sys1 = model.add_software_system(name="System 1") + sys2 = model.add_software_system(name="System 2") + sys3 = model.add_software_system(name="System 3") + rel1 = sys1.uses(sys2) + rel2 = sys3.uses(sys1) + + view = DerivedView(software_system=sys1, description="") + view._add_element(sys1, False) + view._add_element(sys2, False) + view._add_element(sys3, False) + assert view.relationship_views == set() + + view._add_relationships(sys1) + assert len(view.relationship_views) == 2 + assert rel1 in [vr.relationship for vr in view.relationship_views] + assert rel2 in [vr.relationship for vr in view.relationship_views] + + +def test_missing_json_description_allowed(): + """ + Ensure that missing descriptions in the JSON form are supported. + + Raised as https://github.com/Midnighter/structurizr-python/issues/40, it is + permitted through the Structurizr UI to create views with a blank description, + which then gets ommitted from the workspace JSON, so this needs to be allowed by + the Pydantic validation also. + """ + + json = """ + { + "key": "System1-SystemContext" + } + """ + io = ViewIO.parse_raw(json) + assert io is not None