DataStore rework
#53
DataStore rework
#53
Conversation
… PreparePostObject
…pdate permission data
DaniElectra
left a comment
DaniElectra
left a comment
There was a problem hiding this comment.
I like the design this is taking, I think we can build something great from here!
Marked as a draft for now, since this is nowhere near the final version of this PR. Creating the PR now, despite the early state, mostly just to communicate some design goals and start actually tracking progress, as well as to discuss things like optimizing database queries
Just to be clear, what is the scope of the PR? Does it intend to implement every method and functionality, or only some of them and add more methods and functionality on a future PR?
The new philosophy takes inspiration from DaniElectra's MatchMaking changes. DataStore is now opinionated on how it stores and manages data, opting for a standard database schema using Postgres, while exporting some functionality back to the developer for fine-tuned modifications should they need them.
I thought we were going to allow custom methods for database too, as per #37 ? I'm not against this, though, just mentioning it
It seems like newer versions of NEX no longer track the "reference" data? Modern titles like Super Mario Maker do not track it, but older ones like Animal Crossing: New Leaf do (we track it at all times)
I need a refreshal about this, is the "reference" data from before the amiibo update? While AC:NL originally released with NEX 2.7, it got updated with the amiibo update to NEX 3.10, and thus newer than SMM in that aspect
GetNewArrivedNotifications
Gets the list of IDs of notifications created since the input ID. Seems to delete notifications made before the input ID once called? A notification can also be linked to data ID 0 in some cases. Seems to be used by BOSS/HPP
The Data ID 0 notifications are triggered when the console calls GetNotificationUrl, I was able to test this before the shutdown.
| common_globals "github.com/PretendoNetwork/nex-protocols-common-go/v2/globals" | ||
| ) | ||
|
|
||
| func EnableObject(manager *common_globals.DataStoreManager, dataID types.UInt64) *nex.Error { |
There was a problem hiding this comment.
I think we want to add godoc comments on these database funcitons, a short explanation would work
There was a problem hiding this comment.
I was holding off on writing comments here since I know that the database functions are going to change over time
…verifyObjectPermission
Thank you!! Ideally once this is implemented, we should never have to worry about DataStore again (outside of custom implementations)
Ideally, all of them. The foundation for them all is already here, and several of them are "duplicates" of other methods (all the It shouldn't be too much effort to get these all in here tbh, especially since I've also left notes for all the methods. Once they're in here we can start doing some real testing with something like AC:NL
I would still like to do this at some point, but right now the database API isn't even finalized. Like I mentioned earlier, this is highly unoptimized and ideally several of these database functions get reworked or removed entirely to reduce database strain. My priority right now is getting this functional, and then worrying about customization afterwards
My hunch is that "reference" data stopped being tracked either in NEX 3.0 or 3.5 (which is where many changes seems to have occured). AC:NL, despite being updated to 3.10, still tracked this however. So I suspect it's just some jank that AC:NL does specifically (like it's weird permissions). Either than or it's a configurable thing, but I've seen no modern Wii U titles use the "reference" feature. It shouldn't hurt to track it anyway though
I may rely on you a bit for the HPP methods tbh, since I was only able to do a small amount of testing with these and you've looked into them more than I have |
| accessPassword := rand.Uint64() | ||
| updatePassword := rand.Uint64() |
There was a problem hiding this comment.
Hi there, just a small note. This causes the insert to fail ~50% of the time:
[DataStore::Unknown] sql: converting argument $24 type: uint64 values with high bit set are not supported
see also lib/pq#72.
It's unlikely this will ever be a problem on sequential uint64's (hitting the limit there would require a ridiculous amount of data), but it's far more likely here where it's a random number
There was a problem hiding this comment.
I didn't realize this wasn't mentioned here yet, my apologies. We found this out about a week or so ago as well and came up with a solution on Discord. I had thought I brought the info over here, but it seems I did not. To copy-paste from Discord:
the solutions I've seen are:
- Stay within the signed range (not helpful)
- Use a different data type entirely (suggestions like `bytea`, `uuid`, etc. all as hacks)
- Use strings instead of numbers directly
The last one still seems kinda hacky, but it DOES work when using `numeric` columns. If you use `bigint` columns then you run into `value "some giant number" is out of range for type bigint`, but `numeric(20)` accepts a string up to `math.MaxUint64` just fine, and it works just like any other numeric value
So we should probably just add a `Value` function to each of our unsigned types that just returns the result of the types `String` function and call it a day (thats what I did for the uint64 test at least, seems to work fine). We'd need to cast native unsigned integers to ours for this, but that's w/e
DaniElectra
left a comment
DaniElectra
left a comment
There was a problem hiding this comment.
Can we also rebase the branch to fix the conflict with the main branch?
| ) | ||
|
|
||
| func AddUserRating(manager *common_globals.DataStoreManager, dataID types.UInt64, slot types.UInt8, pid types.PID, ratingValue types.Int32) *nex.Error { | ||
| // TODO - Check rows affected? |
There was a problem hiding this comment.
Why would we want to check the rows affected here?
| ) | ||
|
|
||
| func DeleteObjectRatings(manager *common_globals.DataStoreManager, dataID types.UInt64) *nex.Error { | ||
| // TODO - Should we just LOGICALLY delete these, like we do everything else, or continue to HARD delete them? |
There was a problem hiding this comment.
I think we may want to delete these logically to keep track of any cheaters trying to set maximum or minimum ratings for their advantage
| rs.initial_value | ||
| `, dataID, slot).Scan(&ratingInfo.TotalValue, &ratingInfo.Count, &ratingInfo.InitialValue) | ||
| if err != nil { | ||
| if err.Error() == "sql: no rows in result set" { |
There was a problem hiding this comment.
| if err.Error() == "sql: no rows in result set" { | |
| if err == sql.ErrNoRows { |
| $1, | ||
| $2, | ||
| $3, | ||
| $4 |
There was a problem hiding this comment.
| $1, | |
| $2, | |
| $3, | |
| $4 | |
| $1, | |
| $2, | |
| $3 |
| ) | ||
|
|
||
| func UpdateUserRating(manager *common_globals.DataStoreManager, dataID types.UInt64, slot types.UInt8, pid types.PID, ratingValue types.Int32) *nex.Error { | ||
| // TODO - Check rows affected? |
There was a problem hiding this comment.
Why would we want to check the rows affected here?
| func (commonProtocol *CommonProtocol) getObjectInfos(err error, packet nex.PacketInterface, callID uint32, dataIDs types.List[types.UInt64]) (*nex.RMCMessage, *nex.Error) { | ||
| if err != nil { | ||
| common_globals.Logger.Error(err.Error()) | ||
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") |
There was a problem hiding this comment.
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") | |
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, err.Error()) |
| func (commonProtocol *CommonProtocol) getRating(err error, packet nex.PacketInterface, callID uint32, target datastore_types.DataStoreRatingTarget, accessPassword types.UInt64) (*nex.RMCMessage, *nex.Error) { | ||
| if err != nil { | ||
| common_globals.Logger.Error(err.Error()) | ||
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") |
There was a problem hiding this comment.
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") | |
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, err.Error()) |
| func (commonProtocol *CommonProtocol) getRatingWithLog(err error, packet nex.PacketInterface, callID uint32, target datastore_types.DataStoreRatingTarget, accessPassword types.UInt64) (*nex.RMCMessage, *nex.Error) { | ||
| if err != nil { | ||
| common_globals.Logger.Error(err.Error()) | ||
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") |
There was a problem hiding this comment.
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") | |
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, err.Error()) |
| func (commonProtocol *CommonProtocol) getRatings(err error, packet nex.PacketInterface, callID uint32, dataIDs types.List[types.UInt64], accessPassword types.UInt64) (*nex.RMCMessage, *nex.Error) { | ||
| if err != nil { | ||
| common_globals.Logger.Error(err.Error()) | ||
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") |
There was a problem hiding this comment.
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, "change_error") | |
| return nil, nex.NewError(nex.ResultCodes.DataStore.Unknown, err.Error()) |
Well the same on each function, not gonna repeat it on each instance
| return nex.NewError(nex.ResultCodes.DataStore.PermissionDenied, "change_error") | ||
| } | ||
| _, err = manager.Database.Exec(`CREATE TABLE datastore.ratings ( | ||
| id serial PRIMARY KEY, |
There was a problem hiding this comment.
We should use bigserial here for good measure
| id serial PRIMARY KEY, | |
| id bigserial PRIMARY KEY, |
Notifications are used by HPP to share objects with specific access recipients and notify them about it. One such example is letters on Swapdoodle. Implement the necessary functionality and methods to support it. On the database side, add a new table `datastore.notifications` with the following columns: - `id`: The notification ID - `read`: Whether the notification has been read with the `GetNewArrivedNotifications` method - `recipient_id`: The target PID of the notification - `data_id`: The referal data ID of the notification. Can be zero on notifications triggered with `GetNotificationURL`, so don't add an actual reference - `sender_pid`: The PID who triggered the notification. For tracking purposes since this may not always be the owner (for example, on update notifications triggered by an external PID who has the update password) - `creation_date`: When the notification was triggered. For tracking purposes - `read_date`: If the notification has been read, timestamp of that event. For tracking purposes Changes on the S3 side are also required, since we want to store the notification files on a separate key than object files. For this, add a separate `NotifyKeyBase` to specify the location for notification updates, as well as using a different presign function `PresignNotify` for these files for API compatibility with the original design. Alongside that, we need a method for actually putting files into S3 from the server itself. For this, a new `PutObject` is added into `MinIOPresigner` to keep API compatibility. I don't exactly like how this is done, so I'm open to making more changes such as renaming the `S3Presigner` interface to expand its scope in name.
| // DataStoreManager manages a DataStore instance | ||
| type DataStoreManager struct { | ||
| Database *sql.DB | ||
| Endpoint *nex.PRUDPEndPoint |
There was a problem hiding this comment.
This adds a dependency on PRUDP, which wouldn't work for HPP. Since this isn't being used anyway, it should be removed
Resolves #XXX
Changes:
Complete overhaul of the
DataStoreimplementation.Marked as a draft for now, since this is nowhere near the final version of this PR. Creating the PR now, despite the early state, mostly just to communicate some design goals and start actually tracking progress, as well as to discuss things like optimizing database queries
Details on the PR below
Current method progress:
Details
General tasks
statusisDATA_STATUS_PENDINGorDATA_STATUS_REJECTEDregardless of password/permission settingsPhilosophy
The current
DataStoreimplementation opts to only provide a common boilerplate for NEX methods. It leaves the actual logic up to the developer still. The original philosophy was to be as unopinionated as possible, and let each server define the functionality they need independently. However most titles will use nearly the same logic, which has proved to be less than helpful in most cases.The new philosophy takes inspiration from @DaniElectra's MatchMaking changes.
DataStoreis now opinionated on how it stores and manages data, opting for a standard database schema using Postgres, while exporting some functionality back to the developer for fine-tuned modifications should they need them.Notes
Below are some key notes about the implementation and where to go from here:
GetMeta, where the columns change dynamically based on the request. For this reason, a series of very basic builders namedSelectBuilderandUpdateBuilderare provided to build SQL queries dynamically. There are reasons for providing our own:PreparePostObjectif one operation succeeds (such as the object insertion) but later operation fails (such as the rating slot initialization) then data MAY become out of synctransactionalfield, which tell the server to either commit all changes as they happen (transactional=false) or use a "all-or-nothing" approach (transactional=true). Rollback/transactions are needed for these casesextraDatafield seen on several types, which the official servers seem to ignore entirely (meaning I got 0 feedback during live testing prior to the shutdown)DeleteObject)GetMetas) of data will returnDataStore::InvalidArgumentif the length of the input list(s) is(are) greater thanMAX_SEARCH_RESULT_SIZE(100)RateObjectsWithPostingcan only handle 16 pieces of data at once (BATCH_PROCESSING_CAPACITY_POST_OBJECT?)ChangeMetas) must have their input list lengths match exactly, otherwise returnDataStore::InvalidArgumentpResults(or similar) field will (usually) not throw errors, and will instead populate errors insidepResults. If the response also contains real response data (such asRateObjects), the list is populated with zeroed structs for where there are errors (as to mappResultscorrectly)PrepareGetObject,PrepareUpdateObjectandTouchObject, the object has been "referenced"referredTimeis updated to the time of the request (except when usingPrepareUpdateObject)referredCntis incremented by 1 (except when usingPrepareUpdateObject)DATA_FLAG_PERIOD_FROM_LAST_REFERREDis set, the objects expiration time is increased by the objectsperioddays starting from the current timeDATA_FLAG_PERIOD_FROM_LAST_REFERREDis not set, do not update the expiration time. UnlessPrepareUpdateObjectis used, then the objects expiration time is increased by the objectsperioddays starting from the current timeMethod details (delete from here as methods get implemented)
PrepareGetObjectV1
Permission level: Access
Same as
PrepareGetObject. Older version which uses a smaller data ID rangePreparePostObjectV1
Same as
PreparePostObject. Older version which uses a smaller data ID rangeCompletePostObjectV1
Permission level: Owner
Same as
CompletePostObject. Older version which uses a smaller data ID rangeDeleteObject
Permission level: Update
Deletes an object. Anyone with update permissions can delete the object. The exception is objects whose data IDs fall between 900,000-999,999. No one can delete these regardless of permission settings
DeleteObjects
Permission level: Update
Same as
DeleteObject, but for multiple objects. Iftransactionalis true, then deletions are "all or nothing", rolled back if there are any errors.pResultsstores the success/error codesChangeMetaV1
Permission level: Update
Same as
ChangeMeta. Older version which uses a smaller data ID rangeChangeMetasV1
Permission level: Update
Same as
ChangeMetas. Older version which uses a smaller data ID rangeGetMetas
Permission level: Access
Same as
GetMeta, but for multiple objects using the same parameters.pMetaInfowill contain zeroed entries for objects which had errors.pResultsstores the success/error codesPrepareUpdateObject
Permission level: Update
Increments the object's version number and returns an S3 presigned
POSTresponse.DataStore::OperationNotAllowedis the object does not use the file serverCompleteUpdateObject
Permission level: Owner
Tells the server that an object update was a success or not
SearchObject
Permission level: Access
Complex, fine-grained, searches for objects in the database. Covers objects which both do and do not use the storage server. Details on
DataStoreSearchParam:GetNotificationUrl
Exchange an old notification URL for a current one? Seems to be used by BOSS/HPP
GetNewArrivedNotificationsV1
Same as
GetNewArrivedNotifications. Older version which uses a smaller data ID rangeRateObject
Permission level: Access
Rates a specific objects rating slot. The following rating rules apply:
RATING_FLAG_MODIFIABLE, each user only has a single rating for the slot. Update this value and create a logRATING_FLAG_ROUND_MINUS, round the rating to 0 if below 0RATING_FLAG_DISABLE_SELF_RATING, returnDataStore::OperationNotAllowedif the rater owns the objectRATING_INTERNAL_FLAG_USE_RANGE_MIN, returnDataStore::InvalidArgumentif the rating value is below the configured minimum (DataStoreRatingInitParam.rangeMin)RATING_INTERNAL_FLAG_USE_RANGE_MAX, returnDataStore::InvalidArgumentif the rating value is above the configured maximum(DataStoreRatingInitParam.rangeMax)RATING_LOCK_NONE, apply no locks. The user may freely update this slot again at any timeRATING_LOCK_INTERVAL,DataStoreRatingInitParam.periodDurationis used as the number of seconds to lock the user from rating this slot again.DataStoreRatingInitParam.periodDurationmust be positiveRATING_LOCK_PERIOD:DataStoreRatingInitParam.periodDurationis used as the day of the week/month the lock expiresDataStoreRatingInitParam.periodHouris used as the hour of the selected day (0-23)DataStoreRatingInitParam.periodDurationcan be one of:RATING_LOCK_PERIOD_DAY1= -17. The 1st day of the following monthRATING_LOCK_PERIOD_SUN= -7. Sunday of the following weekRATING_LOCK_PERIOD_SAT= -6. Saturday of the following weekRATING_LOCK_PERIOD_FRI= -5. Friday of the following weekRATING_LOCK_PERIOD_THU= -4. Thursday of the following weekRATING_LOCK_PERIOD_WED= -3. Wednesday of the following weekRATING_LOCK_PERIOD_TUE= -2. Tuesday of the following weekRATING_LOCK_PERIOD_MON= -1. Monday of the following weekDataStoreRatingInitParam.periodDurationis set toRATING_LOCK_PERIOD_TUEandDataStoreRatingInitParam.periodHouris set to 0, and a user rates the slot on March 7th 2024, the lock expiration time is set to 12-3-2024 0:00:00 (the following Tuesday at midnight)DataStoreRatingInitParam.periodHouris sent as a signed value, so it may be possible to do negative hours? For example, settingperiodDuration=RATING_LOCK_PERIOD_DAY1andperiodHour=-1to target the last day of the current month?RATING_LOCK_PERMANENT, the user can never rate the slot againRATING_LOCK_NONE, create a logGetRating
Permission level: Access
Gets the rating data for a specific rating slot on a single object
GetRatings
Permission level: Access
Gets all the rating data for every slot for all input data IDs.
pRatingswill contain zeroed entries for objects which had errors.pResultsstores the success/error codesResetRating
Permission level: Update
Resets a specific rating slot data. Rating logs are cleared, rating count is set to 0, and the rating total value is set to the initial value
ResetRatings
Permission level: Update
Resets all the rating data for every slot for all input data IDs. Rating logs are cleared, rating counts are set to 0, and the rating total values are set to their slots initial value. If
transactionalis true, then deletions are "all or nothing", rolled back if there are any errorsGetSpecificMetaV1
Permission level: Access
Same as
GetSpecificMeta. Older version which uses a smaller data ID rangePostMetaBinary
Creates an object which does not use the file server. If the size is anything besides 0, return
DataStore::InvalidArgument. IfDATA_FLAG_NOT_USE_FILESERVERis not set on the object flags on upload, the server sets it automaticallyTouchObject
Permission level: Access
Simulates
PrepareGetObject? Seemingly used to update object expiration times and/or to cheaply check if an object existsGetRatingWithLog
Permission level: Access
Gets the rating data for a specific rating slot on a single object, as well as a log of the current users last rating (if exists). A log is only created if:
RATING_FLAG_MODIFIABLEis usedRATING_LOCK_NONEis usedIf neither case is true then a log cannot be created for a slot, return
DataStore::OperationNotAllowedIf a user has not rated the objects slot,
DataStoreRatingLog.isRatedis false andDataStoreRatingLog.ratingValueis -1If
RATING_FLAG_MODIFIABLEis used and the user has rated the slot,DataStoreRatingLog.isRatedis true andDataStoreRatingLog.ratingValueis the users current rating valueIf a rating lock is used and the user has rated the slot,
DataStoreRatingLog.isRatedis true,DataStoreRatingLog.ratingValueis the users most recent rating value, andDataStoreRatingLog.lockExpirationTimeis the lock expiration dateDataStoreRatingLog.pidis always the users PIDGetNewArrivedNotifications
Gets the list of IDs of notifications created since the input ID. Seems to delete notifications made before the input ID once called? A notification can also be linked to data ID 0 in some cases. Seems to be used by BOSS/HPP
GetSpecificMeta
Permission level: Access
Gets only a subset of data about an object
GetPersistenceInfo
Permission level: Access
Maps a PID and persistence slot to a data ID
GetPersistenceInfos
Permission level: Access
Same as
GetPersistenceInfo, but for multiple objects.pPersistenceInfowill contain zeroed entries for objects which had errors.pResultsstores the success/error codesPerpetuateObject
Permission level: Owner
Marks an object as persisted. If an object occupies the slot already, and
deleteLastObjectis false, the old object has its expiration timer started. If an object occupies the slot already, anddeleteLastObjectis true, the old object is deletedUnperpetuateObject
Permission level: Owner
Stops persisting an object. If
deleteLastObjectis false, the object has its expiration timer started. IfdeleteLastObjectis true, the object is deletedPrepareGetObjectOrMetaBinary
Permission level: Access
Likely used by the client to download data for an object regardless of if it uses the file server or not in a single request. If using the file server, populate
pReqGetInfo. If not using the file server, populatepReqGetAdditionalMetaGetPasswordInfo
Permission level: Owner
Returns an object's access and update passwords. Can only be called by the owner of the object. Passwords bypass normal permissions checks (besides owner permissions)
GetPasswordInfos
Permission level: Owner
Same as
GetPasswordInfo, but for multiple objects.pPasswordInfoswill contain zeroed entries for objects which had errors.pResultsstores the success/error codesGetMetasMultipleParam
Permission level: Access
Similar to
GetMetas, but for multiple objects using the different parametersCompletePostObjects
Permission level: Owner
Same as
CompletePostObject, but for multiple objects. "All or nothing" updatesChangeMetas
Permission level: Update
Same as
ChangeMeta, but for multiple objects.dataIdsandparamsmust be the same length, as they index into each otherRateObjects
Permission level: Access
Same as
RateObject, but for multiple objects.pRatingswill contain zeroed entries for objects which had errors.pResultsstores the success/error codesPostMetaBinaryWithDataId
Based on the name and some code within Xenoblade, seems to upload an object using a specific data ID? We should stub this for now until we have more information
PostMetaBinariesWithDataId
Same as
PostMetaBinaryWithDataId, but for multiple objectsRateObjectWithPosting
Permission level: Access
Will rate an object if data exists, and if not will create the data. For example, if an object was uploaded with only rating slots 1 and 3, and then
RateObjectWithPostingis called targeting slot 2, slot 2 is both initialized and then subsequently rated in this single callRateObjectsWithPosting
Permission level: Access
Same as
RateObjectWithPosting, but for multiple objects.targets,rateParamsandpostParamsmust be the same length, as they index into each otherGetObjectInfos
Permission level: Access
Same as
PrepareGetObject, but for multiple objects.pInfoswill contain zeroed entries for objects which had errors.pResultsstores the success/error codesSearchObjectLight
Permission level: Access
Identical to
SearchObject? Possibly just a slightly search implementation, or uses a cache or something?