revert iterating over history on restore (#4654)

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
This commit is contained in:
Mark Herwege
2025-03-18 21:48:19 +01:00
committed by GitHub
parent 2eec58a95f
commit 195cc3d54d
3 changed files with 41 additions and 62 deletions
@@ -101,9 +101,10 @@ public interface QueryablePersistenceService extends PersistenceService {
/**
* Returns a {@link PersistedItem} representing the persisted state, last update and change timestamps and previous
* persisted state. This can be used to restore the full state of an item.
* The default implementation queries the service and iterates backward to find the last change and previous
* persisted state. Persistence services can override this default implementation with a more specific or efficient
* algorithm.
* The default implementation only queries the service for the last persisted state and does not attempt to look
* further back to find the last change timestamp and previous persisted state. This avoids potential performance
* issues if this method is not overriden in the specific persistence service. Persistence services should override
* this default implementation with a more complete and efficient algorithm.
*
* @param itemName name of item
* @param alias alias of item
@@ -112,51 +113,21 @@ public interface QueryablePersistenceService extends PersistenceService {
*/
default @Nullable PersistedItem persistedItem(String itemName, @Nullable String alias) {
State currentState = UnDefType.NULL;
State previousState = null;
ZonedDateTime lastUpdate = null;
ZonedDateTime lastChange = null;
int pageNumber = 0;
FilterCriteria filter = new FilterCriteria().setItemName(itemName).setEndDate(ZonedDateTime.now())
.setOrdering(Ordering.DESCENDING).setPageSize(1000).setPageNumber(pageNumber);
Iterable<HistoricItem> items = query(filter, alias);
while (items != null) {
Iterator<HistoricItem> it = items.iterator();
int itemCount = 0;
if (UnDefType.NULL.equals(currentState) && it.hasNext()) {
HistoricItem historicItem = it.next();
itemCount++;
currentState = historicItem.getState();
lastUpdate = historicItem.getTimestamp();
lastChange = lastUpdate;
}
while (it.hasNext()) {
HistoricItem historicItem = it.next();
itemCount++;
if (!historicItem.getState().equals(currentState)) {
previousState = historicItem.getState();
items = null;
break;
}
lastChange = historicItem.getTimestamp();
}
if (itemCount == filter.getPageSize()) {
filter.setPageNumber(++pageNumber);
items = query(filter);
} else {
items = null;
}
}
if (UnDefType.NULL.equals(currentState) || lastUpdate == null) {
.setOrdering(Ordering.DESCENDING).setPageSize(1).setPageNumber(0);
Iterator<HistoricItem> it = query(filter, alias).iterator();
if (it.hasNext()) {
HistoricItem historicItem = it.next();
currentState = historicItem.getState();
lastUpdate = historicItem.getTimestamp();
} else {
return null;
}
final State state = currentState;
final ZonedDateTime lastStateUpdate = lastUpdate;
final State lastState = previousState;
// if we don't find a previous state in persistence, we also don't know when it last changed
final ZonedDateTime lastStateChange = previousState != null ? lastChange : null;
return new PersistedItem() {
@@ -177,12 +148,12 @@ public interface QueryablePersistenceService extends PersistenceService {
@Override
public @Nullable ZonedDateTime getLastStateChange() {
return lastStateChange;
return null;
}
@Override
public @Nullable State getLastState() {
return lastState;
return null;
}
};
}
@@ -570,13 +570,33 @@ public class PersistenceManagerImpl implements ItemRegistryChangeListener, State
return;
}
GenericItem genericItem = (GenericItem) item;
if (!UnDefType.NULL.equals(item.getState())) {
State state = item.getState();
State lastState = null;
ZonedDateTime lastStateUpdate = null;
ZonedDateTime lastStateChange = null;
if (UnDefType.NULL.equals(state)) {
state = persistedItem.getState();
lastState = persistedItem.getLastState();
lastStateUpdate = persistedItem.getTimestamp();
lastStateChange = persistedItem.getLastStateChange();
} else {
// someone else already restored the state or a new state was set
return;
// try restoring the previous state if not yet set
if (item.getLastState() != null && item.getLastState() != UnDefType.NULL) {
// there is already a previous state, nothing to restore
return;
}
lastStateUpdate = item.getLastStateUpdate();
if (state.equals(persistedItem.getState())) {
lastState = persistedItem.getLastState();
lastStateChange = persistedItem.getLastStateChange();
} else {
lastState = persistedItem.getState();
lastStateChange = item.getLastStateChange();
}
}
genericItem.removeStateChangeListener(PersistenceManagerImpl.this);
genericItem.setState(persistedItem.getState(), persistedItem.getLastState(), persistedItem.getTimestamp(),
persistedItem.getLastStateChange());
genericItem.setState(state, lastState, lastStateUpdate, lastStateChange);
genericItem.addStateChangeListener(PersistenceManagerImpl.this);
if (logger.isDebugEnabled()) {
logger.debug("Restored item state from '{}' for item '{}' -> '{}'",
@@ -103,7 +103,6 @@ public class PersistenceManagerTest {
private static final GroupItem TEST_GROUP_ITEM = new GroupItem(TEST_GROUP_ITEM_NAME);
private static final State TEST_STATE = new StringType("testState1");
private static final State TEST_LAST_STATE = new StringType("testState2");
private static final HistoricItem TEST_HISTORIC_ITEM = new HistoricItem() {
@Override
@@ -140,12 +139,12 @@ public class PersistenceManagerTest {
@Override
public @Nullable ZonedDateTime getLastStateChange() {
return ZonedDateTime.now().minusDays(2);
return null;
}
@Override
public @Nullable State getLastState() {
return TEST_LAST_STATE;
return null;
}
};
@@ -362,20 +361,15 @@ public class PersistenceManagerTest {
verify(readyServiceMock, timeout(1000)).markReady(any());
assertThat(TEST_ITEM.getState(), is(TEST_STATE));
assertThat(TEST_ITEM.getLastState(), is(TEST_LAST_STATE));
assertThat(TEST_ITEM2.getState(), is(TEST_STATE));
assertThat(TEST_ITEM2.getLastState(), is(TEST_LAST_STATE));
assertThat(TEST_GROUP_ITEM.getState(), is(TEST_STATE));
assertThat(TEST_GROUP_ITEM.getLastState(), is(TEST_LAST_STATE));
verify(queryablePersistenceServiceMock, times(3)).persistedItem(any(), any());
ZonedDateTime lastStateUpdate = TEST_ITEM.getLastStateUpdate();
assertNotNull(lastStateUpdate);
assertTrue(lastStateUpdate.isAfter(ZonedDateTime.now().minusDays(2)));
ZonedDateTime lastStateChange = TEST_ITEM.getLastStateChange();
assertNotNull(lastStateChange);
assertTrue(lastStateChange.isBefore(ZonedDateTime.now().minusDays(2)));
assertTrue(lastStateUpdate.isBefore(ZonedDateTime.now().minusDays(1)));
verifyNoMoreInteractions(queryablePersistenceServiceMock);
verifyNoMoreInteractions(persistenceServiceMock);
@@ -393,20 +387,14 @@ public class PersistenceManagerTest {
verify(readyServiceMock, timeout(1000)).markReady(any());
assertThat(TEST_ITEM.getState(), is(initialValue));
assertThat(TEST_ITEM.getLastState(), is(UnDefType.NULL));
assertThat(TEST_ITEM2.getState(), is(TEST_STATE));
assertThat(TEST_ITEM2.getLastState(), is(TEST_LAST_STATE));
assertThat(TEST_GROUP_ITEM.getState(), is(TEST_STATE));
assertThat(TEST_GROUP_ITEM.getLastState(), is(TEST_LAST_STATE));
verify(queryablePersistenceServiceMock, times(2)).persistedItem(any(), any());
ZonedDateTime lastStateUpdate = TEST_ITEM.getLastStateUpdate();
assertNotNull(lastStateUpdate);
assertTrue(lastStateUpdate.isAfter(ZonedDateTime.now().minusDays(1)));
ZonedDateTime lastStateChange = TEST_ITEM.getLastStateChange();
assertNotNull(lastStateChange);
assertTrue(lastStateChange.isAfter(ZonedDateTime.now().minusDays(1)));
assertTrue(lastStateUpdate.isAfter(ZonedDateTime.now().minusHours(1)));
verifyNoMoreInteractions(queryablePersistenceServiceMock);
verifyNoMoreInteractions(persistenceServiceMock);