Review API methods which do not return a Future+Response type.

[lokka30]

We may also want to review methods which do not return a future-response, like Currency#getStartingBalance(Account), and Account#retrieveName. The aforementioned methods most likely require database queries by economy providers and we are forcing those to be done synchronously.

Methods to Review

• Account

  • [ ] retrieveName

• Currency

  • [ ] getStartingBalance(Account)

• This list is incomplete. Please suggest methods to be added, if you find any which may benefit from using a Future+Response wrapper.

[MrIvanPlays]

Account name shall be retrieved upon account request, as for getStartingBalance this should simply be taken from a config file.

[lokka30]

Account name shall be retrieved upon account request

Initially, I would agree. However - what if it is updated on another server? The value would not synchronise until the economy provider manually updates the data.

As for getStartingBalance this should simply be taken from a config file.

If the starting balance were not influenced by a given context then I would agree to leave it as-is. For economy providers which do something with the given context (instead of ignoring it), they'll need some more time for database operations.

[MrIvanPlays]

However - what if it is updated on another server?

good point, however if treasury is ran by so-called "networks" it should be ran on bungeecord/velocity instances directly, hence this concern is semi-invalid. it's also a sorta "not our problem" concern.

If the starting balance were not influenced by a given context then I would agree to leave it as-is. For economy providers which do something with the given context (instead of ignoring it), they'll need some more time for database operations.

give me a fair enough reason where you'd need to do a database operation to get a starting balance of a specific account.

[lokka30]

however if treasury is ran by so-called "networks" it should be ran on bungeecord/velocity instances directly

I don't think we should require server networks to set up special infrastructure via plugin messaging or something so that they operate via a single economy provider connection.

I also believe that Treasury should not dictate how data is fundamentally stored, i.e. in this case, we are forcing some things to be cached.

It would be easier for everyone if we used our Future+Response system wherever economy provider data is concerned.

give me a fair enough reason where you'd need to do a database operation to get a starting balance of a specific account.

I assume most economy providers will not care about the account context whenever it retrieves the starting balance for a currency. However, those that do will need to fetch data regarding the given account to dictate what starting balance it has for a currency. This may or may not be in-memory, the latter case would make it more suitable for the starting balance method to use a Future+Response.

[lokka30]

@MrIvanPlays, seeking your reply whenever you're free. Thank you

[MrIvanPlays]

I don't think we should require server networks to set up special infrastructure via plugin messaging or something so that they operate via a single economy provider connection.

If treasury is installed globally on a network (or as in installed on bungeecord/velocity), the economy is global. Meaning that plugins, utilizing treasury, are written to run on bungeecord/velocity and DO NOT operate locally on a server. Hence, no plugin messaging and stuff. If there is a local server plugin requiring economy data then this is not our problem.

I assume most economy providers will not care about the account context whenever it retrieves the starting balance for a currency. However, those that do will need to fetch data regarding the given account to dictate what starting balance it has for a currency. This may or may not be in-memory, the latter case would make it more suitable for the starting balance method to use a Future+Response.

Data such as?

[lokka30]

• None of our APIs should force people to store data in any specific way, whether or not we believe that is the best way.
• It's possible an implementation doesn't want an internal cache: they may use an external cache anyways
• Treasury should not force economy providers to run proxy-only on server networks. The economy provider can choose that.

  • If there is a local server plugin requiring economy data then this is not our problem.

  • Yes, it is our problem. We are forcing people to configure their server networks a certain way and telling them that they have to figure out how to fork all of their plugins to hook with the proxy-provided economy. Instead, we can let the server owner decide how they want to configure things (without forcing them any way) and let them deal with their own decision.
• Consistency: all other Treasury data is not forcibly cached internally, what makes this data any different?

Data such as?

That's their decision. Such data may or may not be cached, so why are we assuming that a context-aware method has all of the data it needs in an instant?

[MrIvanPlays]

None of our APIs should force people to store data in any specific way, whether or not we believe that is the best way.

And when did I state that the API is forcing specific storage type?

Treasury should not force economy providers to run proxy-only on server networks. The economy provider can choose that.

Treasury in 99% of the cases won't run on proxies because of it's economy API, more like it's gonna run because of the permissions API (or some other API). If a so-called 'network' wants a global economy and want to utilize treasury for it, then they shall think of how will they set it up. We do not and shall not have responsibility or care how server owners and economy provider developers or any plugin using the treasury api do it. What you're trying to state here is that you want some kind of messaging system in between treasury- and treasury- where you'd need to have treasury installed both on the proxy and on all the underlying servers in order to somehow retain compatibility with plugins utilizing treasury server side. That is imo completely inefficient and waste of resources where the treasury- will serve no purpose if a database is used.

Yes, it is our problem. We are forcing people to configure their server networks a certain way and telling them that they have to figure out how to fork all of their plugins to hook with the proxy-provided economy. Instead, we can let the server owner decide how they want to configure things (without forcing them any way) and let them deal with their own decision.

And in what way are we forcing anyone? It's an API. Implement it however you want, just follow the spec (spec as in what data you shall return).

Consistency: all other Treasury data is not forcibly cached internally, what makes this data any different?

Think of it: 99% of the time you will query accounts based on their name and NOT on their identifier (no matter whether that identifier is an uuid or a namespacedkey). Hence, that data doesn't need to be stored in any way (stored as in being in a map/list/whatever), it is just naturally gonna be attached to the account object itself. AND before you start memory concerning here, a string stored in DRAM is no more than a few bytes (if longer can go up to 1KB and no more). As for getStartingBalance, I cannot think of a reason why do you need an account context for this, but I'll just let it slide, I cba, and so for that my opinion, I don't think of any data that you'd need in order to determine an account's starting balance (unless you're kangarko and hardcode the provider so that when you join some server that uses your economy provider it gives you a million of that currency).

That's their decision.

Ahhh the classics. When we don't have any suitable case to offer, "That's their decision.". I want fair cases, not just empty concerns.

[lokka30]

Thanks for the reply,

And when did I state that the API is forcing specific storage type?

By requiring some data to be internally cached by design due to not having a Future-Response wrapper. Account name, starting balance, etc.

Treasury in 99% of the cases won't run on proxies because of it's economy API, more like it's gonna run because of the permissions API (or some other API).

Agreed. However, do note that our API will allow proxy plugins to finally have decent access to economies. This wasn't readily available beforehand :)

If a so-called 'network' wants a global economy and want to utilize treasury for it, then they shall think of how will they set it up. We do not and shall not have responsibility or care how server owners and economy provider developers or any plugin using the treasury api do it.

Exactly! However, I quote you saying above, "meaning that plugins, utilizing treasury, are written to run on bungeecord/velocity and DO NOT operate locally on a server". You have stated (to my understanding) that a server network should not interface with the Treasury API via its servers, only via proxy/ies. Please address this contradiction.

On that note, remember that multi-proxy environments exist. Internal caching is not compatible with multi-proxy environments, nor network environments. The data can become desynchronised and cause issues - which, in the case of economies, permissions, and other possible systems - could be severe.

you're trying to state here is that you want some kind of messaging system in between treasury- and treasury- where you'd need to have treasury installed both on the proxy and on all the underlying servers in order to somehow retain compatibility with plugins utilizing treasury server side. That is imo completely inefficient and waste of resources where the treasury- will serve no purpose if a database is used.

I never suggested a plugin messaging system, and that is absolutely not within the scope of Treasury. We're on the same page there 🙂

Yes, it is our problem. We are forcing people to configure their server networks a certain way and telling them that they have to figure out how to fork all of their plugins to hook with the proxy-provided economy. Instead, we can let the server owner decide how they want to configure things (without forcing them any way) and let them deal with their own decision.

And in what way are we forcing anyone?

By requiring data to be internally cached by API design, as described in this reply above.

It's an API. Implement it however you want, just follow the spec (spec as in what data you shall return).

Exactly as it should be.

Think of it: 99% of the time you will query accounts based on their name and NOT on their identifier (no matter whether that identifier is an uuid or a namespacedkey).

Correct.

Hence, that data doesn't need to be stored in any way (stored as in being in a map/list/whatever), it is just naturally gonna be attached to the account object itself.

As described above, this is internal caching which will cause synchronisation issues when there are multiple services attached to a given database.

AND before you start memory concerning here, a string stored in DRAM is no more than a few bytes (if longer can go up to 1KB and no more).

I'm absolutely not concerned about a string. :smile:

As for getStartingBalance, I cannot think of a reason why do you need an account context for this, but I'll just let it slide, I cba, and so for that my opinion, I don't think of any data that you'd need in order to determine an account's starting balance (unless you're kangarko and hardcode the provider so that when you join some server that uses your economy provider it gives you a million of that currency).

In the current system, different accounts may be given different starting balances for a given currency. Can't remember why it was added, possibly something I came up with whilst writing Polyconomy.

Yes it would be very useful for kangarko. 😂

If we should remove the account context from getStartingBalance then let me know, I can look into why it was added in the first place.

However I still strongly believe starting balance should use a Future-Response-wrapped return type.

[MrIvanPlays]

By requiring some data to be internally cached by design due to not having a Future-Response wrapper. Account name, starting balance, etc.

And how is that forcing a specific storage type?

You have stated (to my understanding) that a server network should not interface with the Treasury API via its servers, only via proxy/ies. Please address this contradiction.

If treasury's installed on a proxy then it is not expected for it to interact with the servers that are handled by that proxy. Does a luckperms installation on proxy interact with luckperms installations on servers? Of course not. Bungee/Velocity has it's own permissions, and bukkit/spigot/whatever have their own. They're 2 separate systems. Again, everyone's free to implement stuff however they want, it is not and shall not be our problem.

On that note, remember that multi-proxy environments exist. Internal caching is not compatible with multi-proxy environments, nor network environments. The data can become desynchronised and cause issues - which, in the case of economies, permissions, and other possible systems - could be severe. As described above, this is internal caching which will cause synchronisation issues when there are multiple services attached to a given database.

And how is that our problem?

However I still strongly believe starting balance should use a Future-Response-wrapped return type.

And I still strongly do not understand the reasoning behind that, excluding some empty concerns.

[lokka30]

And how is that forcing a specific storage type?

By not returning a Future we are implying that the data should be immediately accessible (i.e. cached in server's memory).

My argument is that the data being discussed in this issue (starting balance, account name, etc) should not be forcefully designated as immediately-accessible data. This design choice will freeze the server if retrieving those values requires a database call or something - which is why I am saying we are practically forcing implementations to locally cache this data, even though this is not suitable for all situations (i.e. server networks as I discussed earlier).

If treasury's installed on a proxy then it is not expected for it to interact with the servers that are handled by that proxy.

Why do we make that choice as an API?

... everyone's free to implement stuff however they want, it is not and shall not be our problem.

Exactly, so we shouldn't be making the choice to practically force implementations to have local caching for non-future-wrapped methods.

And how is that our problem?

Treasury is an API and in its current state we are disregarding a use case for - in my opinion - unwarranted reasons.

And I still strongly do not understand the reasoning behind that, excluding some empty concerns.

Let's say we force retrievePermissions to not return a future-wrapped value. What issues will arise?

• Database operations are likely required, which take a lot of time
  • Since we're not using a future-wrapped value, this will freeze the server until it's completed
• Consistency: everything else uses a future-wrapped value, what makes retrievePermissions any different?
• If there are two or more economy services connected to the same dataset, and both services implement their own internal caching each, the internal cache has to be kept up-to-date with the dataset. Without this parity logic, the services will become desynchronised with the dataset which could result in catastrophic consequences.

These reasons are the exact same for why we must use a Future-wrapped value for account names, starting balances, and whatever else should be using a future.

[MrIvanPlays]

This design choice will freeze the server if retrieving those values requires a database call or something - which is why I am saying we are practically forcing implementations to locally cache this data, even though this is not suitable for all situations (i.e. server networks as I discussed earlier).

And how retrieving a name will freeze the server? As I already mentioned, 99% of the times a name will be provided when retrieving an account and not any identifier, hence you've already got the name. Or if you don't have it, then you will look it up through that specific platform's APIs. That said, if I follow your point, then we have to change every single API method which does not return Future+Response and make it kind of hard to work with them because of the "it is not suitable for all situations" concern, which from the example provided, are a few very niche cases.

Why do we make that choice as an API?

We really don't. It's not a choice. It's basically how every proxy plugin operates. In our case, treasury just provides the API so providers don't have to shade it in (and in bukkit's case, provide placeholders for placeholderapi, in the future in sponge's case implement sponge's economy api), and some useful commands. I still believe you subconsciously want some kind of connection between the proxy installation of treasury and the server installations of treasury.

Let's say we force retrievePermissions to not return a future-wrapped value.

You're comparing apples to oranges here. Right now we're talking about some BigDecimal values and some strings, not about a map that stores permissions, uuids, etc. (lot more stuff).

[lokka30]

I appreciate the time you have taken to discuss this with me, though I don't think we're getting anywhere past misunderstandings. I'll forfeit and leave the current system in unless you wish to change it prior to v2's release.