Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Fixing Issue #353 Capture config users&roles by name #354

Conversation

alencruz
Copy link
Contributor

The setup:get-configuration function in setup.xqy used only ids to
return users and roles. This prevented the capture configurations
option from capturing users and roles specified by name in the
current project configuration or as a parameter. This was a
documented “TODO” in the code in order to improve the capture
functionality. The setup:get-configuration function will now take
user/role names or ids as a parameter and return the requested
configurations. Also added default users and roles from current
project to capture_environment_config in server_config.rb. Lastly
removed word “full” from logging since it could be a full or partial
configuration that is being captured.

The setup:get-configuration function in setup.xqy used only ids to
return users and roles. This prevented the capture configurations
option from capturing users and roles specified by name in the
current project configuration or as a parameter. This was a
documented “TODO” in the code in order to improve the capture
functionality. The setup:get-configuration function will now take
user/role names or ids as a parameter and return the requested
configurations. Also added default users and roles from current
project to capture_environment_config in server_config.rb. Lastly
removed word “full” from logging since it could be a full or partial
configuration that is being captured.
@dmcassel
Copy link
Collaborator

Looks good (haven't tested). A future enhancement would be to pull in the names from the merged configuration rather than the properties, as there are sometimes users or roles put directly into ml-config.xml.

@alencruz
Copy link
Contributor Author

Simplified implementation based on Geert's comments, thanks Geert!

declare function setup:get-users-by-name($names as xs:string*) as element(sec:users)? {
let $ids :=
for $name in $names
return xdmp:user($name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid my advise has had a negative side effect. You former approach would probably have silently ignored users and roles that didn't exist, whereas this will throw a large stacktrace. I lean towards ignoring it silently, you will notice soon enough it didn't capture such users/roles once you look inside the captured ml-config..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I will make change to absorb when users and roles don't exists. Thanks!

@dmcassel
Copy link
Collaborator

Is this PR still in progress?

@alencruz
Copy link
Contributor Author

My apologies, I had not seen the last comments from Geert, my first foray into GitHub, I will look over all comments within the next couple of days for both of my pull requests and report back findings/commit fixes as needed.

@alencruz
Copy link
Contributor Author

alencruz commented Apr 3, 2015

Committed changes based on feedback.

@grtjn
Copy link
Contributor

grtjn commented Apr 14, 2015

Capture now runs without errors..

grtjn added a commit that referenced this pull request Apr 14, 2015
…les-by-name

Fixing Issue #353 Capture config users&roles by name
@grtjn grtjn merged commit 128590f into marklogic-community:dev Apr 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants