Skip to content

Commit b9c87f1

Browse files
authored
Fix session get-or-default (#5662)
The comments described this get-or-default, but instead it was a panic --------- Signed-off-by: Nicholas Gates <[email protected]>
1 parent dcd42d2 commit b9c87f1

File tree

1 file changed

+39
-12
lines changed

1 file changed

+39
-12
lines changed

vortex-session/src/lib.rs

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,20 @@ pub trait SessionExt: Sized + private::Sealed {
6060
fn session(&self) -> VortexSession;
6161

6262
/// Returns the scope variable of type `V`, or inserts a default one if it does not exist.
63-
fn get<V: SessionVar>(&self) -> Ref<'_, V>;
63+
fn get<V: SessionVar + Default>(&self) -> Ref<'_, V>;
64+
65+
/// Returns the scope variable of type `V` if it exists.
66+
fn get_opt<V: SessionVar>(&self) -> Option<Ref<'_, V>>;
6467

6568
/// Returns the scope variable of type `V`, or inserts a default one if it does not exist.
6669
///
6770
/// Note that the returned value internally holds a lock on the variable.
68-
fn get_mut<V: SessionVar>(&self) -> RefMut<'_, V>;
71+
fn get_mut<V: SessionVar + Default>(&self) -> RefMut<'_, V>;
72+
73+
/// Returns the scope variable of type `V`, if it exists.
74+
///
75+
/// Note that the returned value internally holds a lock on the variable.
76+
fn get_mut_opt<V: SessionVar>(&self) -> Option<RefMut<'_, V>>;
6977
}
7078

7179
mod private {
@@ -79,13 +87,12 @@ impl SessionExt for VortexSession {
7987
}
8088

8189
/// Returns the scope variable of type `V`, or inserts a default one if it does not exist.
82-
fn get<V: SessionVar>(&self) -> Ref<'_, V> {
90+
fn get<V: SessionVar + Default>(&self) -> Ref<'_, V> {
8391
Ref(self
8492
.0
85-
.get(&TypeId::of::<V>())
86-
.unwrap_or_else(|| {
87-
vortex_panic!("Session has not been initialized with {}", type_name::<V>())
88-
})
93+
.entry(TypeId::of::<V>())
94+
.or_insert_with(|| Box::new(V::default()))
95+
.downgrade()
8996
.map(|v| {
9097
(**v)
9198
.as_any()
@@ -94,16 +101,25 @@ impl SessionExt for VortexSession {
94101
}))
95102
}
96103

104+
fn get_opt<V: SessionVar>(&self) -> Option<Ref<'_, V>> {
105+
self.0.get(&TypeId::of::<V>()).map(|v| {
106+
Ref(v.map(|v| {
107+
(**v)
108+
.as_any()
109+
.downcast_ref::<V>()
110+
.vortex_expect("Type mismatch - this is a bug")
111+
}))
112+
})
113+
}
114+
97115
/// Returns the scope variable of type `V`, or inserts a default one if it does not exist.
98116
///
99117
/// Note that the returned value internally holds a lock on the variable.
100-
fn get_mut<V: SessionVar>(&self) -> RefMut<'_, V> {
118+
fn get_mut<V: SessionVar + Default>(&self) -> RefMut<'_, V> {
101119
RefMut(
102120
self.0
103-
.get_mut(&TypeId::of::<V>())
104-
.unwrap_or_else(|| {
105-
vortex_panic!("Session has not been initialized with {}", type_name::<V>())
106-
})
121+
.entry(TypeId::of::<V>())
122+
.or_insert_with(|| Box::new(V::default()))
107123
.map(|v| {
108124
(**v)
109125
.as_any_mut()
@@ -112,6 +128,17 @@ impl SessionExt for VortexSession {
112128
}),
113129
)
114130
}
131+
132+
fn get_mut_opt<V: SessionVar>(&self) -> Option<RefMut<'_, V>> {
133+
self.0.get_mut(&TypeId::of::<V>()).map(|v| {
134+
RefMut(v.map(|v| {
135+
(**v)
136+
.as_any_mut()
137+
.downcast_mut::<V>()
138+
.vortex_expect("Type mismatch - this is a bug")
139+
}))
140+
})
141+
}
115142
}
116143

117144
/// A TypeMap based on `https://docs.rs/http/1.2.0/src/http/extensions.rs.html#41-266`.

0 commit comments

Comments
 (0)