Yevgeny Zaspitsky has posted comments on this change. Change subject: engine: Add ManagementNetworkUtil ......................................................................
Patch Set 5: (5 comments) http://gerrit.ovirt.org/#/c/32992/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtil.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtil.java: Line 28: cluster > Typo- cluster cluster Done http://gerrit.ovirt.org/#/c/32992/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtilImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtilImpl.java: Line 22: return getNetworkDao().getManagementNetwork(clusterId); Line 23: } Line 24: Line 25: @Override Line 26: public boolean isManagementNetwork(Guid networkId) { > Consider replacing this code with stored procedure. the logic here is relatively simple. I'm not sure that it worse creating the SP. Line 27: final List<NetworkCluster> networkClusters = getNetworkClusterDao().getAllForNetwork(networkId); Line 28: final NetworkCluster managementNetworkCluster = Line 29: LinqUtils.firstOrNull(networkClusters, new Predicate<NetworkCluster>() { Line 30: @Override http://gerrit.ovirt.org/#/c/32992/5/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtilImplTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/ManagementNetworkUtilImplTest.java: Line 45: private ArgumentCaptor<NetworkClusterId> networkClusterIdCaptor; Line 46: Line 47: @Before Line 48: public void setUp() throws Exception { Line 49: Mockito.when(mockDbFacade.getNetworkDao()).thenReturn(mockNetworkDao); > Please do a static import to avoid the "Mockito." everywhere. Done Line 50: Mockito.when(mockDbFacade.getNetworkClusterDao()).thenReturn(mockNetworkClusterDao); Line 51: Line 52: DbFacadeLocator.setDbFacade(mockDbFacade); Line 53: Line 48: public void setUp() throws Exception { Line 49: Mockito.when(mockDbFacade.getNetworkDao()).thenReturn(mockNetworkDao); Line 50: Mockito.when(mockDbFacade.getNetworkClusterDao()).thenReturn(mockNetworkClusterDao); Line 51: Line 52: DbFacadeLocator.setDbFacade(mockDbFacade); > Please see http://gerrit.ovirt.org/#/c/30663/9/backend/manager/modules/bll/ that will be eliminated in a further change (CDI dependent) Line 53: Line 54: underTest = new ManagementNetworkUtilImpl(); Line 55: } Line 56: Line 116: get > Consider using eq(new(TEST_NETWORK_ID, TEST_CLUSTER_ID)) and verify the get Done -- To view, visit http://gerrit.ovirt.org/32992 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4117f9e97e721c847f5192e1ab724c8d231ce4f3 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
