Move the code to restor values from SoilIndexedDictionary to SoilInde…

…xIterator fixes #545
ApptiveGrid · Dec 19, 2023 · 638275e · 638275e
1 parent 5908586
commit 638275e
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 83 deletions.
diff --git a/src/Soil-Core/SoilIndexIterator.class.st b/src/Soil-Core/SoilIndexIterator.class.st
@@ -5,7 +5,9 @@ Class {
 		'index',
 		'currentPage',
 		'currentKey',
-		'nextKey'
+		'nextKey',
+		'soil',
+		'readVersion'
 	],
 	#category : #'Soil-Core-Index-Common'
 }
@@ -29,8 +31,10 @@ SoilIndexIterator >> at: aKeyObject [
 
 { #category : #accessing }
 SoilIndexIterator >> at: aKeyObject ifAbsent: aBlock [
+	| foundValue |
 	currentKey := (aKeyObject asSkipListKeyOfSize: index keySize) asInteger.
-	^self find: currentKey ifAbsent: aBlock
+	foundValue := self find: currentKey ifAbsent: aBlock.
+ 	^(self restoreValue: foundValue forKey: currentKey) ifNil: [ aBlock value ]
 ]
 
 { #category : #accessing }
@@ -71,8 +75,10 @@ SoilIndexIterator >> currentPage: anObject [
 { #category : #enumerating }
 SoilIndexIterator >> do: aBlock [
 	| item |
-	[ (item := self nextPresent) isNil ] whileFalse: [
-			aBlock value: item ]
+	[ (item := self nextAssociation ) notNil ] whileTrue: [ 
+ 				(self
+ 					 restoreValue: item value
+ 					 forKey: item key) ifNotNil: [:notNil | aBlock value: notNil ]]
 ]
 
 { #category : #private }
@@ -123,6 +129,27 @@ SoilIndexIterator >> firstAssociation [
 	^ item
 ]
 
+{ #category : #private }
+SoilIndexIterator >> historicValueAt: key ifAbsent: absentBlock [
+ 	"a removed value will return ObjectId 0:0"
+ 	readVersion ifNil: [ ^ absentBlock value ].
+ 	^ (self currentPage isOlderThan: readVersion) 
+ 		ifTrue: [ 
+ 			"all modifications to this page have been done before we
+ 			started the transaction, so the removal is visibile and 
+ 			value absent"
+ 			   absentBlock value ] 
+ 		ifFalse: [
+ 			"we determine all changes between our transaction and the
+ 			last one modifying the page. if we get back changes for the
+ 			key the value of the oldes entry has the value it had before"
+ 			(self 
+ 				journalEntriesFor: key 
+ 				startingAt: self currentPage lastTransaction)
+ 					ifNotEmpty: [:entries | entries last oldValue ]
+ 					ifEmpty: absentBlock]
+]
+
 { #category : #accessing }
 SoilIndexIterator >> index [
 	^index
@@ -133,6 +160,29 @@ SoilIndexIterator >> index: anIndex [
 	index := anIndex
 ]
 
+{ #category : #testing }
+SoilIndexIterator >> isEmpty [
+ 	"all items might be removed and not restorable"
+
+ 	^ self index firstPage items allSatisfy: [ :each |
+ 		  (self restoreValue: each value forKey: each key) isNil ]
+]
+
+{ #category : #private }
+SoilIndexIterator >> journalEntriesFor: key startingAt: anInteger [ 
+	| transactionId entries |
+	entries := OrderedCollection new.
+	transactionId := anInteger.
+	[ transactionId >  readVersion ] whileTrue: [  
+		(soil journal transactionJournalAt: transactionId) entries do: [ :each | 
+			((each class = SoilAddKeyEntry) | (each class = SoilRemoveKeyEntry)) ifTrue: [ 
+				(each key = key) ifTrue: [ 
+					entries add: each ] ]  ].
+		transactionId := transactionId - 1.
+	].
+   ^ entries
+]
+
 { #category : #accessing }
 SoilIndexIterator >> last [
 	^ self lastAssociation value
@@ -229,13 +279,40 @@ SoilIndexIterator >> pageAt: anInteger [
 	^ index store pageAt: anInteger
 ]
 
+{ #category : #accessing }
+SoilIndexIterator >> readVersion [
+
+	^ readVersion
+]
+
+{ #category : #accessing }
+SoilIndexIterator >> readVersion: anObject [
+
+	readVersion := anObject
+]
+
 { #category : #removing }
 SoilIndexIterator >> removeKey: key [ 
 	^ self 
 		at: key 
 		put: (SoilObjectId segment: 0 index: 0)
 ]
 
+{ #category : #private }
+SoilIndexIterator >> restoreValue: value forKey: key [
+	"restore a value that has been removed by a later transaction"
+	^ value isRemoved 
+		ifTrue: [
+			self 
+				historicValueAt: key 
+				ifAbsent: [ nil ] ]
+		ifFalse: [
+			"restore a value that has been overwritten by a later transaction"   
+			self 
+				historicValueAt: key 
+				ifAbsent: [ value ] ]
+]
+
 { #category : #accessing }
 SoilIndexIterator >> size [
 	"We iterate over all elements to get the size. Slow!"
@@ -245,6 +322,18 @@ SoilIndexIterator >> size [
 	^ sum
 ]
 
+{ #category : #accessing }
+SoilIndexIterator >> soil [
+
+	^ soil
+]
+
+{ #category : #accessing }
+SoilIndexIterator >> soil: anObject [
+
+	soil := anObject
+]
+
 { #category : #transactions }
 SoilIndexIterator >> updateCurrentTransaction: anInteger [ 
 	currentPage lastTransaction: anInteger

diff --git a/src/Soil-Core/SoilIndexedDictionary.class.st b/src/Soil-Core/SoilIndexedDictionary.class.st
@@ -70,11 +70,7 @@ SoilIndexedDictionary >> atIndex: anInteger [
 
 { #category : #accessing }
 SoilIndexedDictionary >> basicAt: aString ifAbsent: aBlock [ 
-	| iterator value key |
-	key := (aString asSkipListKeyOfSize: index keySize) asInteger.
-	iterator := self newIterator.
-	value := iterator at: aString ifAbsent: aBlock.
-	^ (self restoreValue: value forKey: key iterator: iterator) ifNil: [ aBlock value ]
+	^ self newIterator at: aString ifAbsent: aBlock
 ]
 
 { #category : #accessing }
@@ -92,14 +88,8 @@ SoilIndexedDictionary >> do: aBlock [
 
 	transaction
 		ifNotNil: [ 
-			| iterator assoc |
-			iterator := self newIterator.
-			[ (assoc := iterator nextAssociation) notNil ] whileTrue: [ 
-				(self
-					 restoreValue: assoc value
-					 forKey: assoc key
-					 iterator: iterator) ifNotNil: [ :objectId | 
-					aBlock value: (transaction proxyForObjectId: objectId) ] ] ]
+ 			self newIterator do: [ :objectId | 
+ 					aBlock value: (transaction proxyForObjectId: objectId) ]]
 		ifNil: [ 
 			newValues valuesDo: [ :each | aBlock value: each ] ]
 ]
@@ -138,28 +128,6 @@ SoilIndexedDictionary >> hasIndexUpdates [
 	^ newValues notEmpty or: [ removedValues notEmpty ]
 ]
 
-{ #category : #private }
-SoilIndexedDictionary >> historicValueAt: key iterator: iterator ifAbsent: absentBlock [ 
-	"a removed value will return ObjectId 0:0"
-
-	^ (iterator currentPage isOlderThan: transaction readVersion) 
-		ifTrue: [ 
-			"all modifications to this page have been done before we
-			started the transaction, so the removal is visibile and 
-			value absent"
-			absentBlock value ] 
-		ifFalse: [
-			"we determine all changes between our transaction and the
-			last one modifying the page. if we get back changes for the
-			key the value of the oldes entry has the value it had before"
-			(self 
-				journalEntriesFor: key 
-				startingAt: iterator currentPage lastTransaction)
-					ifNotEmpty: [:entries | entries last oldValue ]
-					ifEmpty: absentBlock ]
-
-]
-
 { #category : #accessing }
 SoilIndexedDictionary >> id [
 	^ id
@@ -186,34 +154,14 @@ SoilIndexedDictionary >> isEmpty [
 	iterator := self newIterator.
 	iterator currentPage: index firstPage.
 	^ newValues isEmpty and: [ index isEmpty 
-		or: [
-		"all items might be removed and not restorable" 
-		(index firstPage items allSatisfy: [ :each | (self
-					 restoreValue: each value
-					 forKey: each key
-					 iterator: iterator)isNil] )]]
+		or: [iterator isEmpty]]
 ]
 
 { #category : #testing }
 SoilIndexedDictionary >> isRegistered [
 	^ index isRegistered 
 ]
 
-{ #category : #private }
-SoilIndexedDictionary >> journalEntriesFor: key startingAt: anInteger [ 
-	| transactionId entries |
-	entries := OrderedCollection new.
-	transactionId := anInteger.
-	[ transactionId > transaction readVersion ] whileTrue: [  
-		(transaction soil journal transactionJournalAt: transactionId) entries do: [ :each | 
-			((each class = SoilAddKeyEntry) | (each class = SoilRemoveKeyEntry)) ifTrue: [ 
-				(each key = key) ifTrue: [ 
-					entries add: each ] ]  ].
-		transactionId := transactionId - 1.
-	].
-   ^ entries
-]
-
 { #category : #accessing }
 SoilIndexedDictionary >> keySize: anInteger [ 
 	index keySize: anInteger 
@@ -326,23 +274,6 @@ SoilIndexedDictionary >> removeKey: key ifAbsent: aBlock [
 				put: (newValues removeKey: key ifAbsent: [ ^ aBlock value ]) ]
 ]
 
-{ #category : #private }
-SoilIndexedDictionary >> restoreValue: value forKey: key iterator: iterator [
-	"restore a value that has been removed by a later transaction"
-	^ value isRemoved 
-		ifTrue: [
-			self 
-				historicValueAt: key 
-				iterator: iterator 
-				ifAbsent: [ nil ] ]
-		ifFalse: [
-			"restore a value that has been overwritten by a later transaction"   
-			self 
-				historicValueAt: key 
-				iterator: iterator 
-				ifAbsent: [ value ] ]
-]
-
 { #category : #accessing }
 SoilIndexedDictionary >> second [
 	^ transaction 
@@ -353,12 +284,7 @@ SoilIndexedDictionary >> second [
 { #category : #accessing }
 SoilIndexedDictionary >> size [ 
 	^ transaction 
-		ifNotNil: [ 
-			"We should just call size on index, but the do: here can restore values"
-			| sum |
-			sum := 0.
-			self do: [ :each | sum := sum + 1 ].
-			sum ]
+		ifNotNil: [ self newIterator size ]
 		ifNil: [ newValues size ]
 ]