[fix] Fixed updating template puts devices in perennial "modified" state #213

[pandafy]

The status of related Config objects will only be updated if the checksum of template's configuration changes.

Closes #213

[coveralls]

coverage: 98.905% (-0.001%) from 98.906% when pulling 7061f76 on issues/213-template-device-modified into f879e84 on master.

[nemesifier]

This is good, thanks!

I was testing on a device and we have a similar problem there, that is, some aspects of the device may be changed (eg: configuration variables) without having any impact on the configuration checksum, and for this reason the config status remains in "modified" state.

After looking at the code I think we could use a similar solution there and I think we should take advantage of this chance to proceed and fix the issue also on Device!

[pandafy]

Empty fields are omitted when generating configuration. Hence, the checksum remained same throughout the test which led to a failing test case. I have updated the test case to avoid this issue.

[nemesifier]

It looks good but there's a detail which is bugging me, please see my comment below.

[nemesifier]

Can you please track what these additional 7 queries are doing here?
I wonder if there's any inefficiency here, if there's any we have to create an issue to address it.

[pandafy]

This is the previous version of the code for _check_changes:

openwisp-controller/openwisp_controller/config/base/config.py

Lines 477 to 493 in f879e84

	def _check_changes(self):
	current = self._meta.model.objects.only(
	'backend', 'config', 'context', 'status'
	).get(pk=self.pk)
	if self.backend != current.backend:
	# storing old backend to send backend change signal after save
	self._old_backend = current.backend
	for attr in ['backend', 'config', 'context']:
	if getattr(self, attr) == getattr(current, attr):
	continue
	if self.status != 'modified':
	self.set_status_modified(save=False)
	else:
	# config modified signal is always sent
	# regardless of the current status
	self._send_config_modified_after_save = True
	break

Earlier, we fetched the configuration object from the database (using 1 query) and then compared each field individually for changes

In this patch, we generate the configuration to get the checksum

openwisp-controller/openwisp_controller/config/base/config.py

Lines 477 to 492 in 7061f76

	def _check_changes(self):
	current = self._meta.model.objects.only(
	'backend', 'config', 'context', 'status'
	).get(pk=self.pk)
	if self.backend != current.backend:
	# storing old backend to send backend change signal after save
	self._old_backend = current.backend
	if hasattr(self, 'backend_instance'):
	del self.backend_instance
	if self.checksum != current.checksum:
	if self.status != 'modified':
	self.set_status_modified(save=False)
	else:
	# config modified signal is always sent
	# regardless of the current status
	self._send_config_modified_after_save = True

Generating the configuration requires additional queries (fetching templates, subnet division rules, etc.). Thus, we see an increase in the number of queries.

This method is only executed when the configuration object is saved.