stevegeek / encoded_id

Turn numeric or hex IDs into reversible and human friendly obfuscated strings
MIT License
8 stars 2 forks source link

Add new option to limit the number of IDs that can be encoded together #5

Closed stevegeek closed 1 year ago

stevegeek commented 1 year ago

Add a new option to ReversibleId#initialize which specifies the maximum number of IDs that be encoded into any one encoded_id (max_inputs_per_id)

The option should have a default value (say 32).

The option should be used in #encode, & #encode_hex

If the input size is greater than max_inputs_per_id then raise InvalidInputError with an appropriate message

avcwisesa commented 1 year ago

Hi Steve! I'm trying to work on this but encountered one doubt.

For encode_hex should the max input be applied when an array of hex strings is provided, or should the array of integers produced from the hex be checked?

Currently, I'm working on the assumption that it's the latter. CMIIW

stevegeek commented 1 year ago

Hi Cenna! Thanks for your contribution! Will take a look at the PR asap.

Yeah, you are correct. Sorry the issue wasn't very clear, I didn't clearly think that through.

We want to limit the number of integer values that can be passed in to the hashing algorithm. This is to limit the amount of work that the underlying hashids gem can be made to perform as it's pretty resource intensive right now. I'm working on an improved implementation but in the mean time we simply offer limits on the number/size of inputs.

avcwisesa commented 1 year ago

We want to limit the number of integer values that can be passed in to the hashing algorithm.

Then seems like max_inputs_per_id is not to be applied directly to input given to encode_hex, and instead used to validate the length of the resulting integer array parsed from hex input 🤔

stevegeek commented 1 year ago

Yeah, let's do that. Ie apply limit to the resulting int array from hex_as_integers. It seems odd I guess, as the relationship between the hex inputs and the integers to be encoded is not obvious, but I think for now it should be fine. I will be revisiting the hex encode/decode in future.

avcwisesa commented 1 year ago

I have made the logic adjustment and updated the rbs also :) PTAL

stevegeek commented 1 year ago

Thanks so much @avcwisesa , that was awesome!