module_utils: Consolidate set_owner_if_different & set_group_if_different

This merges implementations of `AnsibleModule.set_owner_if_different()`
& `AnsibleModule.set_group_if_different()` into a single method. The
benefits are
- reduction in code duplication
- makes possible halving the number of calls to `os.lchown()` when the
  copy module sets both owner and group.

There should be no outward change in behavior. The two original methods
become thin wrappers around the new one.

This PR is related to #75741. If both are merged, then the second
benefit above can be realised - by replacing calls in
`copy.chown_recursive()`.

Open questions:

- Should this new method be public (without a leading underscore)?
- Is there a better name?

changelogs/fragments/75742-chown-if-different.yml

@@ -0,0 +1,6 @@
+minor_changes:
+  - >
+    module_utils - Consolidate the implemenations of
+    `AnsibleModule.set_owner_if_different()` &
+    `AnsibleModule.set_group_if_different()`. Provide wrappers to maintain
+    compatibility.

lib/ansible/module_utils/basic.py

@@ -722,9 +722,8 @@ class AnsibleModule(object):
             changed = True
         return changed
 
-    def set_owner_if_different(self, path, owner, changed, diff=None, expand=True):
+    def _chown_if_different(self, path, owner, group, changed, diff=None, expand=True):
-
+        if owner is None and group is None:
-        if owner is None:
             return changed
 
         b_path = to_bytes(path, errors='surrogate_or_strict')
@@ -736,15 +735,26 @@ class AnsibleModule(object):
 
         orig_uid, orig_gid = self.user_and_group(b_path, expand)
         try:
-            uid = int(owner)
+            uid = -1 if owner is None else int(owner)
         except ValueError:
             try:
                 uid = pwd.getpwnam(owner).pw_uid
             except KeyError:
                 path = to_text(b_path)
                 self.fail_json(path=path, msg='chown failed: failed to look up user %s' % owner)
+        try:
+            gid = -1 if group is None else int(group)
+        except ValueError:
+            try:
+                gid = grp.getgrnam(group).gr_gid
+            except KeyError:
+                path = to_text(b_path)
+                self.fail_json(path=path, msg='chgrp failed: failed to look up group %s' % group)
+
+        if orig_uid == uid and orig_gid == gid:
+            return changed
 
-        if orig_uid != uid:
+        if uid not in (-1, orig_uid):
             if diff is not None:
                 if 'before' not in diff:
                     diff['before'] = {}
@@ -752,40 +762,9 @@ class AnsibleModule(object):
                 if 'after' not in diff:
                     diff['after'] = {}
                 diff['after']['owner'] = uid
-
-            if self.check_mode:
-                return True
-            try:
-                os.lchown(b_path, uid, -1)
-            except (IOError, OSError) as e:
-                path = to_text(b_path)
-                self.fail_json(path=path, msg='chown failed: %s' % (to_text(e)))
             changed = True
-        return changed
-
-    def set_group_if_different(self, path, group, changed, diff=None, expand=True):
-
-        if group is None:
-            return changed
 
-        b_path = to_bytes(path, errors='surrogate_or_strict')
+        if gid not in (-1, orig_gid):
-        if expand:
-            b_path = os.path.expanduser(os.path.expandvars(b_path))
-
-        if self.check_file_absent_if_check_mode(b_path):
-            return True
-
-        orig_uid, orig_gid = self.user_and_group(b_path, expand)
-        try:
-            gid = int(group)
-        except ValueError:
-            try:
-                gid = grp.getgrnam(group).gr_gid
-            except KeyError:
-                path = to_text(b_path)
-                self.fail_json(path=path, msg='chgrp failed: failed to look up group %s' % group)
-
-        if orig_gid != gid:
             if diff is not None:
                 if 'before' not in diff:
                     diff['before'] = {}
@@ -793,17 +772,26 @@ class AnsibleModule(object):
                 if 'after' not in diff:
                     diff['after'] = {}
                 diff['after']['group'] = gid
-
-            if self.check_mode:
-                return True
-            try:
-                os.lchown(b_path, -1, gid)
-            except OSError:
-                path = to_text(b_path)
-                self.fail_json(path=path, msg='chgrp failed')
             changed = True
+
+        if self.check_mode:
+            return changed
+
+        try:
+            os.lchown(b_path, uid, gid)
+        except (IOError, OSError) as e:
+            path = to_text(b_path)
+            op = 'chown' if uid != -1 else 'chgrp'
+            self.fail_json(path=path, msg='%s failed: %s' % (op, to_text(e)))
+
         return changed
 
+    def set_owner_if_different(self, path, owner, changed, diff=None, expand=True):
+        return self._chown_if_different(path, owner, None, changed, diff=None, expand=True)
+
+    def set_group_if_different(self, path, group, changed, diff=None, expand=True):
+        return self._chown_if_different(path, None, group, changed, diff=None, expand=True)
+
     def set_mode_if_different(self, path, mode, changed, diff=None, expand=True):
 
         if mode is None: