Bug 30878 - Data representation of interfaces for the frontend
Data representation of interfaces for the frontend
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - Basic settings
UCS 3.0
Other Linux
: P3 enhancement (vote)
: UCS 3.2
Assigned To: Florian Best
Alexander Kläser
: interim-2
Depends on: 28670 33091
Blocks: 30816
  Show dependency treegraph
 
Reported: 2013-03-22 16:31 CET by Alexander Kläser
Modified: 2013-11-19 06:44 CET (History)
2 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Cleanup
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kläser univentionstaff 2013-03-22 16:31:20 CET
Currently, the data representation of interfaces is sent as UCR variables to the UMC frontend. This complicates the handling as an abstraction needs to implemented on the backend side as well as on the frontend side. The goal of this bug is to send information about interfaces not as UCR variables, but as a suitable dict-oriented data structure. For this, the object oriented abstraction developed in Bug 28670 will be used.
Comment 1 Stefan Gohmann univentionstaff 2013-06-11 09:00:19 CEST
On a UCS 3.2 test instance I got the following error during system setup appliance:

__MSG__:Processing triggers for python-support ...
__MSG__:Running post-installation trigger python-support
__STEP__:300.0
__MSG__:Compiling /usr/lib/pymodules/python2.6/univention/management/console/modules/setup/network.py ...
__MSG__:SyntaxError: ('invalid syntax', ('/usr/lib/pymodules/python2.6/univention/management/console/modules/setup/network.py', 72, 13, '\t\t\t\t\tall_ip4s.add(ip4[0])\n'))
__MSG__:
__MSG__:Fetched 0 B in 0s (0 B/s)
Comment 2 Alexander Kläser univentionstaff 2013-06-11 12:48:44 CEST
(In reply to Stefan Gohmann from comment #1)
> On a UCS 3.2 test instance I got the following error during system setup
> appliance:
> 
> __MSG__:Processing triggers for python-support ...
> __MSG__:Running post-installation trigger python-support
> __STEP__:300.0
> __MSG__:Compiling
> /usr/lib/pymodules/python2.6/univention/management/console/modules/setup/
> network.py ...
> __MSG__:SyntaxError: ('invalid syntax',
> ('/usr/lib/pymodules/python2.6/univention/management/console/modules/setup/
> network.py', 72, 13, '\t\t\t\t\tall_ip4s.add(ip4[0])\n'))
> __MSG__:
> __MSG__:Fetched 0 B in 0s (0 B/s)

I opened a new bug for this error → Bug 31710
Comment 3 Alexander Kläser univentionstaff 2013-06-11 14:21:14 CEST
(In reply to Alexander Kläser from comment #2)
> (In reply to Stefan Gohmann from comment #1)
> > On a UCS 3.2 test instance I got the following error during system setup
> > appliance:
> > 
> > __MSG__:Processing triggers for python-support ...
> > __MSG__:Running post-installation trigger python-support
> > __STEP__:300.0
> > __MSG__:Compiling
> > /usr/lib/pymodules/python2.6/univention/management/console/modules/setup/
> > network.py ...
> > __MSG__:SyntaxError: ('invalid syntax',
> > ('/usr/lib/pymodules/python2.6/univention/management/console/modules/setup/
> > network.py', 72, 13, '\t\t\t\t\tall_ip4s.add(ip4[0])\n'))
> > __MSG__:
> > __MSG__:Fetched 0 B in 0s (0 B/s)
> 
> I opened a new bug for this error → Bug 31710

My error, this is a bug that happened during the development for UCS 3.2, I fixed the syntax error, package has been rebuilt:

 univention-system-setup (7.0.2-1) unstable; urgency=low
 .
   * [WIP] Bug #28389: fixed syntax error
Comment 4 Florian Best univentionstaff 2013-07-02 08:52:30 CEST
Implemented in univention.manangement.console.modules.setup.network.
Comment 5 Alexander Kläser univentionstaff 2013-08-02 17:23:00 CEST
Changelog → OK

The Python part is AFAIS fine. In the following, some aspects referring the JS implementation.

(1)
====================
constructor: function(props) {
    this.interfaceType = props.interfaceType;
    this.name = props.name;
    this.ip4 = props.ip4 || [];
    this.ip6 = props.ip6 || [];
    this.ip4dynamic = props.ip4dynamic || false;
    this.ip6dynamic = props.ip6dynamic || false;
    this.primary = props.primary || false;
    this.options = props.options || [];
    this.label = self.interfaceTypes[this.interfaceType];
},
====================
This could be also done via lang.mixin({ip4:..., ...}, props).

(2)
====================
toObject: function() {
    return {
        interfaceType: this.interfaceType,
        name: this.name,
        ip4: this.ip4,
        ip6: this.ip6,
        ip4dynamic: this.ip4dynamic,
        ip6dynamic: this.ip6dynamic,
        primary: this.primary,
        options: this.options
    };
},
====================
This could be done, e.g.:
====================
var obj = {};
array.forEach(["interfaceType", "name", ...], function(i) {
  obj[i] = this[i];
}, this);
====================

(3)
====================
var back = '';
if (back) {
     back += '<br>';
}
====================
Maybe shorter via an array:
====================
var back = [];
...
back.push(...);
...
return back.join('<br>');
====================

(4)
====================
var arr = self.interfaceValues();
if (!self.Bond.getPossibleSubdevices(all_interfaces, physical_interfaces, devicename).length) {
    arr = array.filter(arr, function(i) { return i.id !== 'Bond'; });
}
if (!self.Ethernet.getPossibleSubdevices(all_interfaces, physical_interfaces, devicename).length) {
    arr = array.filter(arr, function(i) { return i.id !== 'Ethernet'; });
}
...
====================
Could be more compact, e.g. via:
====================
var deviceAvailable = {};
array.forEach(['Bond', 'Ethernet', ...], function(itype) {
  var typeAvailable = 0 < self[itype].getPossibleSubdevices(...).length;
  devicesAvailable[itype] = typeAvailable;
}, this);
return array.filter(self.interfaceValues(), function(ientry) {
  return deviceAvailable[ientry.id];
});
====================

(5)
  getPossibleDevices function(all_interfaces, physical_interfaces, devicename)
(a) Why all_interfaces and physical_interfaces? A function that filters out the physical_interfaces from all_interfaces would do the job and would remove on function parameter.
(b) You could also (as in the Python part) add all_interfaces as constructor parameter, such that every device knows all available ones. This would remove the first function parameter.
(c) "devicename" … is this the device to be ignored? Then make the function getPossibleDevices() as non-static method and this parameter can be removed, as well.

(6)
  var availableInterfaces = array.map(all_interfaces, function(item) {
      return item.name;
  });
→ It is better to always work with a list of Interface instances instead of a list of device names! That way you can directly access convenience functions that help to make the code more clear and compact.

(7)
====================
var devices = [];
array.forEach(physical_interfaces, function(device) {
    if (array.every(all_interfaces, function(idevice) {
        if (devicename != idevice.name) {
            return (-1 === idevice.getSubdevices().indexOf(device));
        }
        return true;
    })) {
        devices.push(device);
    }
});
====================
I did not get it directly with a function in the if condition, more explicit with subfunctions isInUse() would be good (taken into account what has been said in the points before):
====================
var devicesNotInUse = array.filter(this.getPhysicalDevices(), function(idev) {
    return idev.name === this.name || !idev.inUse();   
}, this);
...
====================
This should do the job, as well. inUse() needs probably to call inUse() of its subdevices? Not sure.
Comment 6 Alexander Kläser univentionstaff 2013-08-02 17:24:32 CEST
(In reply to Alexander Kläser from comment #5)
> ...
> (1)
> ...
> This could be also done via lang.mixin({ip4:..., ...}, props).

Oops, should be lang.mixin(this, {ip4:..., ...}, props).
Comment 7 Florian Best univentionstaff 2013-08-29 09:58:19 CEST
(In reply to Alexander Kläser from comment #5)
These points are cleaned up.
Comment 8 Alexander Kläser univentionstaff 2013-09-19 14:24:49 CEST
QA: JS device representation looks fine now :) .
Comment 9 Stefan Gohmann univentionstaff 2013-11-19 06:44:27 CET
UCS 3.2 has been released:
 http://docs.univention.de/release-notes-3.2-en.html
 http://docs.univention.de/release-notes-3.2-de.html

If this error occurs again, please use "Clone This Bug".