Alona Kaplan has posted comments on this change. Change subject: webadmin: add profiles instead of networks to vnic and vm dialogs ......................................................................
Patch Set 19: (13 comments) .................................................... File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationMessages.java Line 104: Line 105: @DefaultMessage("default: {0}") Line 106: String defaultTimeZoneCaption(String currentDefault); Line 107: Line 108: @DefaultMessage("VM has {0} Nics. Assign them to the Profiles.") Done Line 109: String assignNicsToProfilesPlural(int numOfNics); Line 110: Line 111: @DefaultMessage("VM has 1 Nic. Assign it to a Profile.") Line 112: String assignNicsToProfilesSingular(); .................................................... File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/network/LogicalNetworkEditor.java Line 18: import com.google.gwt.user.client.TakesValue; Line 19: import com.google.gwt.user.client.ui.Composite; Line 20: import com.google.gwt.user.client.ui.Widget; Line 21: Line 22: public class LogicalNetworkEditor extends Composite implements IsEditor<TakesValueEditor<VnicInstanceType>>, TakesValue<VnicInstanceType>, HasElementId { The items are not going to be changed after initializing the editor. So in this phase it would be an overhead to implement TakesConstrainedValue. And also, this is not in the scope of this patch:) Line 23: Line 24: interface WidgetUiBinder extends UiBinder<Widget, LogicalNetworkEditor> { Line 25: WidgetUiBinder uiBinder = GWT.create(WidgetUiBinder.class); Line 26: } .................................................... File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/networkinterface/NetworkInterfacePopupWidget.java Line 65: Line 66: @UiField(provided = true) Line 67: @Path(value = "profile.selectedItem") Line 68: @WithElementId("profile") Line 69: public ListModelTypeAheadListBoxEditor<Object> profileEditor; The LogicalNetworkEditor is strongly binded to vnic instance type and can't be reused here. Line 70: Line 71: @UiField(provided = true) Line 72: @Path("nicType.selectedItem") Line 73: @WithElementId("nicType") Line 168: } Line 169: Line 170: @SuppressWarnings({ "rawtypes", "unchecked" }) Line 171: private void initManualWidgets() { Line 172: profileEditor = new ListModelTypeAheadListBoxEditor<Object>( Done Line 173: new ListModelTypeAheadListBoxEditor.NullSafeSuggestBoxRenderer<Object>() { Line 174: Line 175: @Override Line 176: public String getReplacementStringNullSafe(Object data) { .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditProfileBehavior.java Line 10: Line 11: public class EditProfileBehavior extends ProfileBehavior { Line 12: Line 13: @Override Line 14: public void initSelectedProfile(ListModel profileList, VmNetworkInterface networkInterface) { This method is called only once when opening the edit dialog. I don't think it worth to maintain a map for it. Line 15: List<VnicProfileView> profiles = (List<VnicProfileView>) profileList.getItems(); Line 16: profiles = profiles == null ? new ArrayList<VnicProfileView>() : profiles; Line 17: Line 18: if (networkInterface.getVnicProfileId() == null) { .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewProfileBehavior.java Line 18: public void initSelectedProfile(ListModel profileList, VmNetworkInterface networkInterface) { Line 19: List<VnicProfileView> profiles = (List<VnicProfileView>) profileList.getItems(); Line 20: profiles = profiles == null ? new ArrayList<VnicProfileView>() : profiles; Line 21: for (VnicProfileView profile : profiles) { Line 22: if (ENGINE_NETWORK_NAME != null && profile != null && profile.getNetworkName() != null 1. The key of the map you"ve suggested is the profile name. Here I need to find the profile by the network name. So the map wouldn't help me here. 2. You are right. Removed this check. Line 23: && ENGINE_NETWORK_NAME.equals(profile.getNetworkName())) { Line 24: profileList.setSelectedItem(profile); Line 25: return; Line 26: } .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ProfileBehavior.java Line 28: profilesQuery.converterCallback = new IAsyncConverter() { Line 29: Line 30: @Override Line 31: public Object Convert(Object returnValue, AsyncQuery asyncQuery) { Line 32: ArrayList<Network> clusterNetworks = (ArrayList<Network>) asyncQuery.getModel(); Done Line 33: Line 34: ProfileBehavior.this.clusterNetworks = clusterNetworks; Line 35: Line 36: ArrayList<VnicProfileView> vnicProfiles = new ArrayList<VnicProfileView>(); Line 38: if (returnValue == null) { Line 39: return vnicProfiles; Line 40: } Line 41: Line 42: for (VnicProfileView vnicProfile : (ArrayList<VnicProfileView>) returnValue) { Changed the cast to List instead of ArrayList. I prefer not to use iterator. I think foreach is much more readable. Line 43: Network network = findNetworkById(vnicProfile.getNetworkId()); Line 44: if (network != null && network.isVmNetwork()) { Line 45: vnicProfiles.add(vnicProfile); Line 46: } Line 50: if (hotUpdateSupported) { Line 51: vnicProfiles.add(null); Line 52: } Line 53: Line 54: Collections.sort(vnicProfiles, new Comparator<VnicProfileView>() { You are right:) That's why I did it in patch http://gerrit.ovirt.org/#/c/17574/ Line 55: Line 56: private LexoNumericComparator lexoNumeric = new LexoNumericComparator(); Line 57: Line 58: @Override .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInterfaceCreatingManager.java Line 44: Line 45: /** Line 46: * Used mainly when the VM is created from the template. If the Vm has been created but without NICs, new Line 47: * ones are created according to the vnicInstanceTypes. If the VM is created with nics - e.g. by copying from template update they're profiles Line 48: * as edited by the user (again, according to the vnicInstanceTypes). Fixed the typo. It is formatted, the problem is with gerrit's display. Line 49: * Line 50: * @param vmId The ID of the VM Line 51: * @param vnicInstanceTypes list of nics as edited in the window Line 52: */ Line 65: // there are some vnics created - update according to the setup in the window Line 66: ArrayList<VdcActionParametersBase> parameters = new ArrayList<VdcActionParametersBase>(); Line 67: Line 68: for (VmNetworkInterface created : createdNics) { Line 69: for (VnicInstanceType edited : vnicInstanceTypes) { This class was renamed from VmNetworkCreatingManager.java to VmInterfaceCreatingManager.java. And was adapted to use profiles. Most of the code in thus class is not in the scope of this patch. Line 70: // can not use getId() because they have different IDs - one is already created, one is not Line 71: // yet Line 72: boolean sameNic = edited.getNetworkInterface().getName().equals(created.getName()); Line 73: Line 68: for (VmNetworkInterface created : createdNics) { Line 69: for (VnicInstanceType edited : vnicInstanceTypes) { Line 70: // can not use getId() because they have different IDs - one is already created, one is not Line 71: // yet Line 72: boolean sameNic = edited.getNetworkInterface().getName().equals(created.getName()); This is not correct. You can create two vnics with the same name and different cases. Line 73: Line 74: boolean bothProfilesNull = created.getVnicProfileId() == null && edited.getNetworkInterface().getVnicProfileId() == null; Line 75: Line 76: boolean sameProfiles = created.getVnicProfileId() != null && created.getVnicProfileId().equals(edited.getNetworkInterface().getVnicProfileId()); .................................................... File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VnicInstanceType.java Line 2: Line 3: import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; Line 4: import org.ovirt.engine.ui.uicommonweb.models.ListModel; Line 5: Line 6: public class VnicInstanceType extends ListModel { It is vnic representation to be used as instance type for the vm. Line 7: Line 8: private VmNetworkInterface networkInterface; Line 9: Line 10: public VnicInstanceType(VmNetworkInterface networkInterface) { -- To view, visit http://gerrit.ovirt.org/17322 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00ea69609acf8a8bfeb0a5357489141f726b6323 Gerrit-PatchSet: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches