Skip to content

Change rbx_reflection_database to support loading from FS #376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7145f07
Add local support to rbx_reflection_database
Dekkonot Nov 13, 2023
a4409b7
Add some logging
Dekkonot Nov 13, 2023
c5bfda2
Update changelog
Dekkonot Nov 13, 2023
ca2cbf0
Update README and module documentation
Dekkonot Nov 13, 2023
f8c833f
remove easily
Dekkonot Nov 13, 2023
69d6a70
Specify that env var points to file, not dir
Dekkonot Nov 14, 2023
6bd1b0b
Correctly refer to environment variables as such
Dekkonot Nov 14, 2023
f61eab3
Update crate documentation to reflect 69d6a70
Dekkonot Nov 14, 2023
d066132
Make `get_local` and `get` use Result
Dekkonot Nov 14, 2023
a9f3beb
Update usage of `get` in other crates
Dekkonot Nov 14, 2023
2413092
Correctly handle local path not existing
Dekkonot Nov 14, 2023
99171cb
Merge branch 'master' into dynamic-reflection-pog
Dekkonot Sep 3, 2024
d1eeac4
Merge branch 'master' into dynamic-reflection-pog
Dekkonot Dec 17, 2024
e5f8c05
Add PR number and link to changelog
Dekkonot Dec 17, 2024
17128fe
Fix database tests
Dekkonot Dec 17, 2024
d95bc80
Undo formatting I did on accident
Dekkonot Dec 17, 2024
d525f03
Use LazyStatic + drop lazy_static as dependency
Dekkonot Jan 2, 2025
7d639f3
Add test to confirm that the location returned by `get_local_location…
Dekkonot Jan 2, 2025
a1d2d37
Allow the `unnecessary_operation` clippy lint
Dekkonot Jan 2, 2025
230c4ae
Handle env vars better
Dekkonot Jan 2, 2025
ec8a046
...Disable the clippy lint correctly?
Dekkonot Jan 2, 2025
7328ded
Accept defeat, make a function that does what we're testing
Dekkonot Jan 2, 2025
bc78a6f
Merge branch 'master' into dynamic-reflection-pog
Dekkonot Apr 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rbx_binary/src/deserializer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<'db> Deserializer<'db> {
/// Create a new `Deserializer` with the default settings.
pub fn new() -> Self {
Self {
database: rbx_reflection_database::get(),
database: rbx_reflection_database::get().unwrap(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion rbx_binary/src/serializer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<'db> Serializer<'db> {
/// Create a new `Serializer` with the default settings.
pub fn new() -> Self {
Serializer {
database: rbx_reflection_database::get(),
database: rbx_reflection_database::get().unwrap(),
compression: CompressionType::default(),
}
}
Expand Down
6 changes: 6 additions & 0 deletions rbx_reflection_database/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# rbx\_reflection_database Changelog

## Unreleased Changes
- The database may now be loaded dynamically from the local file system. ([#376])
The location is OS-dependent but it will only be loaded if one exists. The location may also be manually specified using the `RBX_DATABASE` environment variable.

`get` is unchanged in use, but will return a locally stored database if it exists, and the bundled one if not. Two new methods were added: `get_bundled` will only fetch the local database and `get_local` will only fetch a locally stored one.

[#376]: https://github.com/rojo-rbx/rbx-dom/pull/376

## 1.0.1+roblox-666 (2025-03-31)
* Corrected the serialization of `MeshPart.TextureID` to migrate to `MeshPart.TextureContent` ([#505])
Expand Down
3 changes: 2 additions & 1 deletion rbx_reflection_database/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ edition = "2018"
[dependencies]
rbx_reflection = { version = "5.0.0", path = "../rbx_reflection" }

lazy_static = "1.4.0"
serde = "1.0.137"
rmp-serde = "1.1.1"
dirs = "5.0.1"
log = "0.4.20"
22 changes: 21 additions & 1 deletion rbx_reflection_database/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,24 @@

More details about this crate are available on [the rbx-dom GitHub](https://github.com/rojo-rbx/rbx-dom#readme).

Contains a generated Roblox reflection database using the types from [rbx_reflection](https://crates.io/crates/rbx_reflection). This crate embeds a MessagePack-format database that is exposed through this crate's only method, `get`.
Contains an API to get a Roblox reflection database using the types from [`rbx_reflection`](https://crates.io/crates/rbx_reflection). This crate embeds a database for this purpose, but also provides an API for dependents to get a reflection database from a consistent location.

The general way this crate should be used is via `get`. This method will search for a locally stored reflection database and return it if it's found. If it isn't, it will instead return the bundled one. The details for where it searches are below.

Additionally, this crate exposes `get_local` and `get_bundled` for only loading the locally stored database or only the bundled one respectively.

## Local Details

This crate will load a reflection database from the file system if one exists in the default location. This location varies upon the OS and is specified here:

| OS | Location |
|:--------|:--------------------------------------------------------------------|
| Windows | `%localappdata%/.rbxreflection/database.msgpack` |
| MacOS | `$HOME/Library/Application Support/.rbxreflection/database.msgpack` |
| Linux | `$HOME/.rbxreflection/database.msgpack` |

Additionally, a location override may be specified via the `RBX_DATABASE` environment variable. The `RBX_DATABASE` variable points to the override `database.msgpack` file, _not_ to an override `.rbxreflection` directory.

Both the default `database.msgpack` files and any files pointed to by `RBX_DATABASE` must be valid MessagePack serializations of a [`ReflectionDatabase`][ReflectionDatabase] if they're present.

[ReflectionDatabase]: https://docs.rs/rbx_reflection/latest/rbx_reflection/struct.ReflectionDatabase.html
Binary file added rbx_reflection_database/empty.msgpack
Binary file not shown.
24 changes: 24 additions & 0 deletions rbx_reflection_database/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
use std::{fmt, io};

#[derive(Debug, Clone)]
pub struct Error(String);

impl std::error::Error for Error {}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}

impl From<rmp_serde::decode::Error> for Error {
fn from(value: rmp_serde::decode::Error) -> Self {
Self(value.to_string())
}
}

impl From<io::Error> for Error {
fn from(value: io::Error) -> Self {
Self(value.to_string())
}
}
197 changes: 188 additions & 9 deletions rbx_reflection_database/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,147 @@
//! Contains an API to get a Roblox reflection database using the types
//! from [`rbx_reflection`]. This crate embeds a database for this purpose,
//! but also provides an API for dependents to get a reflection database
//! from a consistent location.
//!
//! The general way this crate should be used is via [`get`]. This method will
//! search for a locally stored reflection database and return it if it's
//! found. If it isn't, it will instead return the bundled one.
//!
//! Additionally, this crate exposes [`get_local`] and [`get_bundled`] for
//! only loading the locally stored database or only the bundled one
//! respectively.
//!
//! ## Local Details
//!
//! This crate will load a reflection database from the file system if one
//! exists in the default location. This location varies upon the OS and is
//! specified here:
//!
//! | OS | Location |
//! |:--------|:--------------------------------------------------------------------|
//! | Windows | `%localappdata%/.rbxreflection/database.msgpack` |
//! | MacOS | `$HOME/Library/Application Support/.rbxreflection/database.msgpack` |
//! | Linux | `$HOME/.rbxreflection/database.msgpack` |
//!
//! Additionally, a location override may be specified via the `RBX_DATABASE`
//! environment variable. The `RBX_DATABASE` variable points to the override
//! `database.msgpack` file, _not_ to an override `.rbxreflection` directory.
//!
//! Both the default `database.msgpack` files and any files pointed to by
//! `RBX_DATABASE` must be valid MessagePack serializations of a
//! [`ReflectionDatabase`] if they're present.
mod error;

use rbx_reflection::ReflectionDatabase;

use std::{env, fs, path::PathBuf, sync::LazyLock};

pub use error::Error;

/// An alias to avoid overly verbose types.
type ResultOption<T> = Result<Option<T>, Error>;

static ENCODED_DATABASE: &[u8] = include_bytes!("../database.msgpack");

lazy_static::lazy_static! {
static ref DATABASE: ReflectionDatabase<'static> = {
rmp_serde::decode::from_slice(ENCODED_DATABASE).unwrap_or_else(|e| panic!("could not decode reflection database because: {}", e))
/// The name of an environment variable that may be used to specify
/// the location of a reflection database to use. The expected format of
/// a file at this point is MessagePack.
pub const OVERRIDE_PATH_VAR: &str = "RBX_DATABASE";

/// The name of the directory used for the local location for a reflection
/// database. The directory will be placed inside the current user's
/// local data folder on MacOS and Windows and inside
/// the home directory on Linux.
pub const LOCAL_DIR_NAME: &str = ".rbxreflection";

static BUNDLED_DATABASE: LazyLock<ReflectionDatabase<'static>> = LazyLock::new(|| {
log::debug!("Loading bundled reflection database");
rmp_serde::decode::from_slice(ENCODED_DATABASE)
.unwrap_or_else(|e| panic!("could not decode reflection database because: {}", e))
});

static LOCAL_DATABASE: LazyLock<ResultOption<ReflectionDatabase<'static>>> = LazyLock::new(|| {
let Some(path) = get_local_location() else {
return Ok(None);
};
if path.exists() {
let database: ReflectionDatabase<'static> = rmp_serde::from_slice(&fs::read(path)?)?;
Ok(Some(database))
} else {
Ok(None)
}
});

/// Returns a populated [`ReflectionDatabase`]. This will attempt to load one locally and
/// if one can't be found, it will return one that is bundled with this crate.
///
/// ## Errors
///
/// Errors if a locally stored [`ReflectionDatabase`] could not be read
/// or is invalid MessagePack.
pub fn get() -> Result<&'static ReflectionDatabase<'static>, Error> {
Ok(get_local()?.unwrap_or(&BUNDLED_DATABASE))
}
Comment on lines +82 to +84
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not totally sure if breaking this interface is the right call. The way I'm thinking about it is: if get_local fails, then probably 99% of the time, users of this function will immediately want to call get_bundled, so why not just have this function do that for them? That way it does the right thing most of the time, and we can avoid breakage.

On the other hand, then it's not clear that this function can fail, and removes the ability to e.g. print a warning if the local database is corrupt while using this function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not totally sold on breaking this either, but the other option is hiding failures from users. It might be worth taking a middleground and just emitting a message into stderr if the local database exists but is corrupted?

It may also be worth caching it if we do this though, so it doesn't print the message every time.


/// Returns a reflection database from the file system, if one can be found.
/// This is loaded from a location set by the `RBX_DATABASE` environment
/// variable if it's set. Otherwise, the default location is checked.
///
/// The default location varies depending upon OS:
///
/// | OS | Location |
/// |:--------|:--------------------------------------------------------------------|
/// | Windows | `%localappdata%/.rbxreflection/database.msgpack` |
/// | MacOS | `$HOME/Library/Application Support/.rbxreflection/database.msgpack` |
/// | Linux | `$HOME/.rbxreflection/database.msgpack` |
///
/// The file at the above location (or the one pointed to by `RBX_DATABASE`)
/// must be valid MessagePack.
///
/// ## Errors
///
/// Errors if the file specified by `RBX_DATABASE` or in the default location
/// exists but is invalid MessagePack.
pub fn get_local() -> ResultOption<&'static ReflectionDatabase<'static>> {
let local_database: &ResultOption<ReflectionDatabase<'static>> = &LOCAL_DATABASE;
match local_database {
Ok(opt) => Ok(opt.as_ref()),
// This clone could be avoided because these references are static,
// but it'd involve some indirection and these errors are rare anyway.
Err(e) => Err(e.clone()),
}
}

pub fn get() -> &'static ReflectionDatabase<'static> {
&DATABASE
/// Returns the locally bundled [`ReflectionDatabase`]. This database may or may
/// not be up to date, but it will always exist.
pub fn get_bundled() -> &'static ReflectionDatabase<'static> {
&BUNDLED_DATABASE
}

/// Fetches the location a [`ReflectionDatabase`] is expected to be loaded from.
/// This may return [`None`] if the local data directory cannot be found.
pub fn get_local_location() -> Option<PathBuf> {
if let Ok(location) = env::var(OVERRIDE_PATH_VAR) {
log::debug!("Using environment variable {OVERRIDE_PATH_VAR} to fetch reflection database");
Some(PathBuf::from(location))
} else {
get_local_location_no_var()
}
}

/// Returns the default local location for the reflection database, without
/// considering the env variable.
fn get_local_location_no_var() -> Option<PathBuf> {
// Due to concerns about the local data directory existing
// on Linux, we use the home directory instead.
#[cfg(target_os = "linux")]
let mut home = dirs::home_dir()?;
#[cfg(not(target_os = "linux"))]
let mut home = dirs::data_local_dir()?;

home.push(LOCAL_DIR_NAME);
home.push("database.msgpack");
Some(home)
}

#[cfg(test)]
Expand All @@ -19,13 +151,60 @@ mod test {
use super::*;

#[test]
fn smoke_test() {
let _database = get();
fn bundled() {
let _database = get_bundled();
}

#[test]
fn env_var() {
let mut test_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
test_path.push("empty.msgpack");

unsafe { env::set_var(OVERRIDE_PATH_VAR, &test_path) };
let empty_db = get().unwrap();
assert!(empty_db.version == [0, 0, 0, 0]);
}

#[test]
fn local_location() {
#[allow(unused_mut, reason = "this path needs to be mutated on macos")]
let mut home_from_env;

#[cfg(target_os = "windows")]
#[allow(
clippy::unnecessary_operation,
reason = "attributes on statements are currently unstable so this cannot be reduced"
)]
{
home_from_env = PathBuf::from(env!("LOCALAPPDATA"));
}
#[cfg(target_os = "macos")]
#[allow(
clippy::unnecessary_operation,
reason = "attributes on statements are currently unstable so this cannot be reduced"
)]
{
home_from_env = PathBuf::from(env!("HOME"));
home_from_env.push("Library");
home_from_env.push("Application Support");
}
#[cfg(not(any(target_os = "windows", target_os = "macos")))]
#[allow(
clippy::unnecessary_operation,
reason = "attributes on statements are currently unstable so this cannot be reduced"
)]
{
home_from_env = PathBuf::from(env!("HOME"))
};
let mut local_expected = home_from_env.join(LOCAL_DIR_NAME);
local_expected.push("database.msgpack");

assert_eq!(get_local_location_no_var().unwrap(), local_expected);
}

#[test]
fn superclasses_iter_test() {
let database = get();
let database = get_bundled();
let part_class_descriptor = database.classes.get("Part");
let mut iter = database.superclasses_iter(part_class_descriptor.unwrap());
fn class_descriptor_eq(lhs: Option<&ClassDescriptor>, rhs: Option<&ClassDescriptor>) {
Expand All @@ -51,7 +230,7 @@ mod test {

#[test]
fn has_superclass_test() {
let database = get();
let database = get_bundled();
let part_class_descriptor = database.classes.get("Part").unwrap();
let instance_class_descriptor = database.classes.get("Instance").unwrap();
assert!(database.has_superclass(part_class_descriptor, instance_class_descriptor));
Expand Down
2 changes: 1 addition & 1 deletion rbx_xml/src/deserializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<'db> DecodeOptions<'db> {
pub fn new() -> Self {
DecodeOptions {
property_behavior: DecodePropertyBehavior::IgnoreUnknown,
database: rbx_reflection_database::get(),
database: rbx_reflection_database::get().unwrap(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion rbx_xml/src/serializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl<'db> EncodeOptions<'db> {
pub fn new() -> Self {
EncodeOptions {
property_behavior: EncodePropertyBehavior::IgnoreUnknown,
database: rbx_reflection_database::get(),
database: rbx_reflection_database::get().unwrap(),
}
}

Expand Down