Beta
Superb resource access control configuration
Loading description...
Recursion
Algorithms
View
This comment has been reported as {{ abuseKindText }}.
Show
This comment has been hidden. You can view it now .
This comment can not be viewed.
- |
- Reply
- Edit
- View Solution
- Expand 1 Reply Expand {{ comments?.length }} replies
- Collapse
- Spoiler
- Remove
- Remove comment & replies
- Report
{{ fetchSolutionsError }}
-
-
Your rendered github-flavored markdown will appear here.
-
Label this discussion...
-
No Label
Keep the comment unlabeled if none of the below applies.
-
Issue
Use the issue label when reporting problems with the kata.
Be sure to explain the problem clearly and include the steps to reproduce. -
Suggestion
Use the suggestion label if you have feedback on how this kata can be improved.
-
Question
Use the question label if you have questions and/or need help solving the kata.
Don't forget to mention the language you're using, and mark as having spoiler if you include your solution.
-
No Label
- Cancel
Commenting is not allowed on this discussion
You cannot view this solution
There is no solution to show
Please sign in or sign up to leave a comment.
Random tests calls user solution twice per test case for some reasons.
Also, mutating the input breaks the random tests.
Reference solution is demonstrably wrong:
[ { '5': 0 } ]
matches the entryand so the result shouldn't be
null
.More specifically, reference solution consistently gives incorrect results when credentials contains a falsy value (which, by how random tests are generated, are always
0
).Reference solution fails on the same case in
deny
too:The tests doesn't specify what separates between a value and a nested key, and what happens if a collision happens, such as having
resource: { value: 1 }
andresource: { value: { nested: 1 } }
together. If this won't happen, this still needs to be specified.There are multiple test assertions in a
it
block, but they aren't distinguished by test failure message, so it's very hard to figure out which test failed inside ait
block.The kata is still woefully under-specified on the actual requirements, and the test block descriptions are very undescriptive about what the actual requirements are. Taking some examples from the sample tests:
What does
if deny admits same types as allow
even mean? Why is the first check denied if it doesn't match{ user: 'guess' }
?Alternatively, why aren't all the requirements listed in the description? TDD is one thing, but having the actual spec scattered in a million test blocks is definitely an issue. TDD is not a substitute for proper specs documentation.
This is completely unspecified: what is the expected behaviour for
denies
object in case of same keys, different keys, or no keys? Is itall
orsome
?The existing tests do not provide enough information to deduce this.
This is nonsensical; earlier the tests have already established that a merge only happens if a key occurs more than once in the config resource: there is only 1 in this case. The actual spec is clearly something else (most probably related to what
childConfig
actually means), which needs to be explained.The meaning of
resource: null
is completely unspecified. Are you trying to make us read your mind?I can understand you don't want to tediously explain each and every configuration option in the description. But it would be nice if you gave a high-level overview of all the gazillion possibilities in the description.
The Example tests do a nice job of building bottom-up, but I like to go into a kata with a top-down initial design, so I hopefully don't have to change the basic design too much to allow for surprise requirements. Normally, I would want the specs in the description. All the specs. But I can see that's not going to work in this case.
Incidentally, I started reading the Example tests after noticing the size of the scroll bar, thus the humongous size of the tests, and made it to two thirds before just plain giving up for now. This kata is a lot more involved and bigger than the description would have you believe ( but for that last little line - I took it with a grain of salt, but it was orders of magnitude bigger than I took it for ).
I have extended the description of the kata a bit, trying to anticipate all the difficulties that you will encounter. I hope that helps.
In the last description example,
why would
guess
get access to{ value: 1 }
?Yes, you're right, it's my mistake. I have fix it. Thank you.
Needs Random Tests.
I understand your point when I see your solution :) Fixed
Totally wrong. You, the author, are responsible for specifying the requirement clearly. Just look at your long long solution, how the hell are we able to guess what you thought? Like
By merging, why
{...:[2,3]}
and{...:1}
got{...:[2,3,1]}
instead of{...:[[2,3],1]}
?['root', [user => user === 'guess', /^s/]]
Seriously? Nested array? How deep could it be? And what should we do if credential value to match is an array?Let's see, the config says that
any users start with 'g' will be denied
, then why{}
is denied? It doesnt even have a user field (maybe it does have aundefined
user field, butundefined
starts with u)What the heck is
!user
?... ...
I usually make a clear description of the requirements, but in this kata I chose to do it this way because I believe it best. I have extended the description a bit by adding more cases and requirements.I have made a great effort so that the tests explain the rest.
Yes, I have decided that it is so. I have added a clarification in the comments.
Because you need to pass an user.
deny
should check than the user key is passed. Any user that does not start with g is valid in this case. But the user key is needed.This is an error that I have already corrected.
Thank you very much for your comments, they have helped me improve the kata.
Hey, I can't read spoilers! And I was also wondering what
!user
was supposed to do ..There's also a
isSUperUser
somewhere - I suppose that's just a typo ( wrt capitalisation ).Sorry, I have mistakenly marked the comment as a spoiler. And, yes, both are typos.
In the description the value of
allowed
property is an object, however in the tests it's an array.allow
can be both: an object or an array of objects. I have clarified it in the comments.It's not a good idea to return either a value or an array of values based on the amount of the final results.
Yes, you're right. I have modified it so that it is always an array.