fix: Address edge cases for excluding large properties when using save #1356
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rties in arrays (#1336) * Add some test cases for findLargeProperties_ * Add a test for arrays and skip it. * Add a test for the complex case wrapped in an arra * Add a test case for an array without name/value. * Add another test for name/value pair * Change the test case involving a name/value pair * Add two simple test cases for excludeFromIndexes * Add another unit test for findLargeProperties_ * Remove if block that stops recursion * Add a test for save with a `name/value` pair * Add some test code that ensures name/value encoded Against the current source code, this test fails because the code throws an error, but it should pass. * Add header * Remove only * Add a test that saves a long string value property * Add a unit test checking longString for value * Remove only * TODO is done
…gleapis/nodejs-datastore into exclude-from-indexes-cleanup
…cludeFromIndexes list (#1349) * Add some test cases for findLargeProperties_ * Add a test for arrays and skip it. * Add a test for the complex case wrapped in an arra * Add a test case for an array without name/value. * Add another test for name/value pair * Change the test case involving a name/value pair * Add two simple test cases for excludeFromIndexes * Add another unit test for findLargeProperties_ * Remove if block that stops recursion * Add a test for save with a `name/value` pair * Add some test code that ensures name/value encoded Against the current source code, this test fails because the code throws an error, but it should pass. * Add header * Remove only * Add a test that saves a long string value property * Add a unit test checking longString for value * Remove only * TODO is done * Refactor the save function Pull out the code that extends the excludeFromIndexes list * Add TODO * Write unit tests for the new excludeFromIndexes fn * Add the header * Document parameter for extendExcludeFromIndexes * Remove redundant check
…into exclude-from-indexes-cleanup
…gleapis/nodejs-datastore into exclude-from-indexes-cleanup
* Add some test cases for findLargeProperties_ * Add a test for arrays and skip it. * Add a test for the complex case wrapped in an arra * Add a test case for an array without name/value. * Add another test for name/value pair * Change the test case involving a name/value pair * Add two simple test cases for excludeFromIndexes * Add another unit test for findLargeProperties_ * Remove if block that stops recursion * Add a test for save with a `name/value` pair * Add some test code that ensures name/value encoded Against the current source code, this test fails because the code throws an error, but it should pass. * Add header * Remove only * Add a test that saves a long string value property * Add a unit test checking longString for value * Remove only * TODO is done * Refactor the save function Pull out the code that extends the excludeFromIndexes list * Add TODO * Write unit tests for the new excludeFromIndexes fn * Add the header * Refactor the code for building the entity proto * Rename it buildEntityProto * The tests need another mock due to the structure The structure change requires another mock. * First two tests for buildEntityProto * Reintroduce extendExcludeFromIndexes.ts tests * Adjust the test to match expected proto * Add another test for an entity in an array * Add headers * Add another header * Eliminate space. Simplify diff
…Proto always excludes large properties from indexing (#1355) * Add some test cases for findLargeProperties_ * Add a test for arrays and skip it. * Add a test for the complex case wrapped in an arra * Add a test case for an array without name/value. * Add another test for name/value pair * Change the test case involving a name/value pair * Add two simple test cases for excludeFromIndexes * Add another unit test for findLargeProperties_ * Remove if block that stops recursion * Add a test for save with a `name/value` pair * Add some test code that ensures name/value encoded Against the current source code, this test fails because the code throws an error, but it should pass. * Add header * Remove only * Add a test that saves a long string value property * Add a unit test checking longString for value * Remove only * TODO is done * Refactor the save function Pull out the code that extends the excludeFromIndexes list * Add TODO * Write unit tests for the new excludeFromIndexes fn * Add the header * Refactor the code for building the entity proto * Rename it buildEntityProto * The tests need another mock due to the structure The structure change requires another mock. * First two tests for buildEntityProto * Reintroduce extendExcludeFromIndexes.ts tests * Adjust the test to match expected proto * Add another test for an entity in an array * Add headers * Add another header * Add some tests for checkEntityProto Make sure that checkEntityProto is working correctly. * Empty commit * Pull out complexCaseEntities * Add a few more tests to excludeIndexesAndBuildProto * Add generated test cases ensuring completeness * Add a few comments that explain behaviour of block * Add comments explaining code blocks * Add a bunch of other generated tests * Make this test more meaningful for name/value chek * Fix the generator * Remove console logs * Add better test name descriptions * Add TODO * Add a comment about the array of arrays test case * Increase the max depth * Add headers to the codebase * Update parameter descriptions * Add a bunch of parameter documentation Correct only as well.
product-auto-label
bot
added
size: xl
daniel-sanche
approved these changes
Nov 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
To solve #1242 a PR was merged. However, some edge cases remain where a valid argument passed into the save function will produce an error when it shouldn't. This PR contains changes that address those edge cases that were already reviewed in the following PRs:
#1336
#1349
#1350
#1355