vehicles: fix regression in GenericStorageInfo

Motivation:

Commit 5d60302 introduced a regression when handling a deserliased
StorageInfo from an earlier version of dCache.  Under these
circumstances, the new field is not initialised and is `null`.

Modification:

Remove the _locationsRO field.  This is an example of premature
optimisation.

Currently, GenericStorageInfo contains a memory optimisation for the
common case where a file has no tape locations.  This implementation is
broken (works-by-accident).

This patch fixes this, so the optimisation is applied uniformly and all
functions work at expected.

Remove various tests the check whether `_locations` is `null`.  There is
no way that `_locations` can be `null`, so these tests are redundant and
only make the code harder to read.

Result:

An unreleased regression is fixed.

Target: master
Requires-notes: no
Requires-book: no
Patch: https://rb.dcache.org/r/13427/
Acked-by: Tigran Mkrtchyan

modules/dcache-vehicles/src/main/java/diskCacheV111/vehicles/GenericStorageInfo.java

@@ -30,8 +30,11 @@ public class GenericStorageInfo
     private RetentionPolicy _retentionPolicy = StorageInfo.DEFAULT_RETENTION_POLICY;
 
     private Map<String, String> _keyHash = new HashMap<>();
-    private List<URI> _locations = new ArrayList<>();
-    private List<URI> _locationsRO = Collections.unmodifiableList(_locations);
+
+    // Use a singleton instance in order to reduce memory footprint for a common case, where a file
+    // has no tape locations.
+    private List<URI> _locations = List.of();
+
     private boolean _setHsm;
     private boolean _setStorageClass;
     private boolean _setBitFileId;
@@ -68,6 +71,9 @@ public void clearKeys() {
 
     @Override
     public void addLocation(URI newLocation) {
+        if (_locations.isEmpty()) {
+            _locations = new ArrayList<>();
+        }
         _locations.add(newLocation);
     }
 
@@ -158,20 +164,17 @@ public void isSetStorageClass(boolean isSet) {
 
     @Override
     public boolean isStored() {
-        /*
-         * FIXME: _locations!= null is needed to read old SI files
-         */
-        return _locations != null && !_locations.isEmpty();
+        return !_locations.isEmpty();
     }
 
     @Override
     public List<URI> locations() {
-        return _locationsRO;
+        return _locations.isEmpty() ? _locations : Collections.unmodifiableList(_locations);
     }
 
     @Override
     public void clearLocations() {
-        _locations.clear();
+        _locations = List.of();
     }
 
     @Override
@@ -232,10 +235,8 @@ public String toString() {
                 sb.append(key).append('=').append(value).append(';');
             }
         }
-        if (_locations != null) {
-            for (URI location : _locations) {
-                sb.append(location).append(';');
-            }
+        for (URI location : _locations) {
+            sb.append(location).append(';');
         }
         return sb.toString();
     }
@@ -338,7 +339,9 @@ public GenericStorageInfo clone() {
         try {
             GenericStorageInfo copy = (GenericStorageInfo) super.clone();
             copy._keyHash = new HashMap<>(_keyHash);
-            copy._locations = new ArrayList<>(_locations);
+            copy._locations = _locations.isEmpty()
+                    ? List.of()
+                    : new ArrayList<>(_locations);
             return copy;
         } catch (CloneNotSupportedException e) {
             throw new RuntimeException("Failed to clone storage info: " +
@@ -431,7 +434,6 @@ private void readObject(java.io.ObjectInputStream stream)
         }
 
         if (_locations == null || _locations.isEmpty()) {
-            // immutable list is ok as only PnfsManager initially populates the locations.
             _locations = List.of();
         }
     }