diff --git a/modules/dcache/src/main/java/org/dcache/pool/repository/RepositoryInterpreter.java b/modules/dcache/src/main/java/org/dcache/pool/repository/RepositoryInterpreter.java index 4f5cf1c2784..8a49620b7cd 100644 --- a/modules/dcache/src/main/java/org/dcache/pool/repository/RepositoryInterpreter.java +++ b/modules/dcache/src/main/java/org/dcache/pool/repository/RepositoryInterpreter.java @@ -324,8 +324,10 @@ private Serializable listAll() throws CacheException, InterruptedException { ReplicaState state = entry.getState(); FileAttributes attributes = entry.getFileAttributes(); - AccessLatency accessLatency = attributes.getAccessLatency(); - RetentionPolicy retentionPolicy = attributes.getRetentionPolicy(); + AccessLatency accessLatency = attributes.isDefined(FileAttribute.ACCESS_LATENCY) + ? attributes.getAccessLatency() : null; + RetentionPolicy retentionPolicy = attributes.isDefined(FileAttribute.RETENTION_POLICY) + ? attributes.getRetentionPolicy() : null; if (siFilter != null) { String siValue = attributes.isDefined(FileAttribute.STORAGECLASS) ? attributes.getStorageClass() @@ -378,14 +380,18 @@ private Serializable listAll() throws CacheException, InterruptedException { private void printQosInfo(CacheEntry entry, StringBuilder sb) throws CacheException, InterruptedException { String format = Strings.nullToEmpty(this.format); FileAttributes attributes = entry.getFileAttributes(); - AccessLatency accessLatency = attributes.getAccessLatency(); - RetentionPolicy retentionPolicy = attributes.getRetentionPolicy(); - if ((format.equalsIgnoreCase("unmanaged") && (accessLatency == AccessLatency.ONLINE && !entry.isSticky()) || + AccessLatency accessLatency = attributes.isDefined(FileAttribute.ACCESS_LATENCY) + ? attributes.getAccessLatency() + : null; + RetentionPolicy retentionPolicy = attributes.isDefined(FileAttribute.RETENTION_POLICY) + ? attributes.getRetentionPolicy() + : null; + if (format.equalsIgnoreCase("unmanaged") && ((accessLatency == AccessLatency.ONLINE && !entry.isSticky()) || (accessLatency == AccessLatency.NEARLINE && retentionPolicy == RetentionPolicy.CUSTODIAL && entry.isSticky()))) { sb.append(" : "); - sb.append(accessLatency).append(" : "); - sb.append(retentionPolicy).append(" : "); + sb.append(accessLatency == null ? "" : accessLatency).append(" : "); + sb.append(retentionPolicy == null ? "" : retentionPolicy).append(" : "); printPinInfo(entry, sb); } @@ -402,8 +408,12 @@ private void printQosInfo(CacheEntry entry, StringBuilder sb) throws CacheExcept private boolean isUnmanaged(CacheEntry entry) throws CacheException, InterruptedException { FileAttributes attributes = entry.getFileAttributes(); - AccessLatency accessLatency = attributes.getAccessLatency(); - RetentionPolicy retentionPolicy = attributes.getRetentionPolicy(); + AccessLatency accessLatency = attributes.isDefined(FileAttribute.ACCESS_LATENCY) + ? attributes.getAccessLatency() + : null; + RetentionPolicy retentionPolicy = attributes.isDefined(FileAttribute.RETENTION_POLICY) + ? attributes.getRetentionPolicy() + : null; return (accessLatency == AccessLatency.ONLINE && !entry.isSticky()) || (accessLatency == AccessLatency.NEARLINE && retentionPolicy == RetentionPolicy.CUSTODIAL && entry.isSticky()); diff --git a/modules/dcache/src/test/java/org/dcache/pool/repository/RepositoryInterpreterTest.java b/modules/dcache/src/test/java/org/dcache/pool/repository/RepositoryInterpreterTest.java new file mode 100644 index 00000000000..6902462d159 --- /dev/null +++ b/modules/dcache/src/test/java/org/dcache/pool/repository/RepositoryInterpreterTest.java @@ -0,0 +1,89 @@ +package org.dcache.pool.repository; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.emptyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import diskCacheV111.util.PnfsId; +import java.util.Collections; +import java.util.List; +import org.dcache.namespace.FileAttribute; +import org.dcache.vehicles.FileAttributes; +import org.junit.Before; +import org.junit.Test; + +/** + * Regression test for https://github.com/dCache/dcache/issues/8134 : 'rep ls' used to crash with + * an IllegalStateException when a replica's FileAttributes had no ACCESS_LATENCY defined. + */ +public class RepositoryInterpreterTest { + + private static final PnfsId PNFSID = + new PnfsId("000000000000000000000000000000000001"); + + private RepositoryInterpreter _interpreter; + private Repository _repository; + + @Before + public void setUp() { + _repository = mock(Repository.class); + _interpreter = new RepositoryInterpreter(); + _interpreter.setRepository(_repository); + } + + @Test + public void repLsDoesNotCrashOnUndefinedAccessLatency() throws Exception { + FileAttributes attributes = new FileAttributes(); + assertThat(attributes.isDefined(FileAttribute.ACCESS_LATENCY), org.hamcrest.Matchers.is(false)); + + CacheEntry entry = mock(CacheEntry.class); + when(entry.getFileAttributes()).thenReturn(attributes); + when(entry.getState()).thenReturn(ReplicaState.CACHED); + when(entry.isSticky()).thenReturn(false); + when(entry.getStickyRecords()).thenReturn(Collections.emptyList()); + when(entry.toString()).thenReturn(PNFSID + " 0 si={}"); + + when(_repository.iterator()).thenReturn(List.of(PNFSID).iterator()); + when(_repository.getEntry(PNFSID)).thenReturn(entry); + + RepositoryInterpreter.ListCommand cmd = _interpreter.new ListCommand(); + cmd.format = "unmanaged"; + + // Must not throw IllegalStateException: Attribute is not defined: ACCESS_LATENCY + String result = (String) cmd.execute(); + + verify(_repository).getEntry(PNFSID); + // Undefined access latency means the replica can't be classified as unmanaged, + // so it is excluded from the report rather than crashing the command. + assertThat(result, emptyString()); + } + + @Test + public void repLsDoesNotCrashOnUndefinedRetentionPolicy() throws Exception { + FileAttributes attributes = new FileAttributes(); + assertThat(attributes.isDefined(FileAttribute.RETENTION_POLICY), org.hamcrest.Matchers.is(false)); + + CacheEntry entry = mock(CacheEntry.class); + when(entry.getFileAttributes()).thenReturn(attributes); + when(entry.getState()).thenReturn(ReplicaState.CACHED); + when(entry.isSticky()).thenReturn(false); + when(entry.getStickyRecords()).thenReturn(Collections.emptyList()); + when(entry.toString()).thenReturn(PNFSID + " 0 si={}"); + + when(_repository.iterator()).thenReturn(List.of(PNFSID).iterator()); + when(_repository.getEntry(PNFSID)).thenReturn(entry); + + RepositoryInterpreter.ListCommand cmd = _interpreter.new ListCommand(); + cmd.format = "unmanaged"; + + // Must not throw IllegalStateException: Attribute is not defined: RETENTION_POLICY + String result = (String) cmd.execute(); + + verify(_repository).getEntry(PNFSID); + // Undefined retention policy means the replica can't be classified as unmanaged, + // so it is excluded from the report rather than crashing the command. + assertThat(result, emptyString()); + } +}