Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Update global role + fixed issue with GUI custom role edition #10612

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
11 changes: 11 additions & 0 deletions doc/release-notes/8808-10575-update-global-role.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
## Release Highlights

### Update a Global Role

A new API endpoint has been added that allows a global role to be updated. See [Native API Guide > Update Global Role](https://guides.dataverse.org/en/6.3/api/native-api.html#update-global-role) (#10612)

## Bug fixes

### Edition of custom role fixed

It is now possible to edit a custom role with the same alias (reported in #8808)
35 changes: 28 additions & 7 deletions doc/sphinx-guides/source/api/native-api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4461,12 +4461,12 @@ The JSON representation of a role (``roles.json``) looks like this::

{
"alias": "sys1",
"name": Restricted System Role,
"description": A person who may only add datasets.,
"name": "Restricted System Role",
"description": "A person who may only add datasets.",
"permissions": [
"AddDataset"
]
}
}

.. note:: alias is constrained to a length of 16 characters

Expand Down Expand Up @@ -5593,22 +5593,43 @@ Creates a global role in the Dataverse installation. The data POSTed are assumed
.. code-block:: bash

export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
export SERVER_URL=https://demo.dataverse.org
export ID=root
export SERVER_URL=http://localhost:8080

curl -H "Content-Type: application/json" -H "X-Dataverse-key:$API_TOKEN" -X POST "$SERVER_URL/api/admin/roles" --upload-file roles.json

``roles.json`` see :ref:`json-representation-of-a-role`

Update Global Role
~~~~~~~~~~~~~~~~~~

Update a global role in the Dataverse installation. The PUTed data is assumed to be a complete JSON role as it will overwrite the existing role. ::

PUT http://$SERVER/api/admin/roles/$ID

curl -H "X-Dataverse-key:$API_TOKEN" -X POST "$SERVER_URL/api/admin/roles" --upload-file roles.json
A curl example using an ``ID``

.. code-block:: bash

export SERVER_URL=http://localhost:8080
export ID=24

curl -H "Content-Type: application/json" -X PUT "$SERVER_URL/api/admin/roles/$ID" --upload-file roles.json

``roles.json`` see :ref:`json-representation-of-a-role`

Delete Global Role
~~~~~~~~~~~~~~~~~~

Deletes an ``DataverseRole`` whose ``id`` is passed. ::

DELETE http://$SERVER/api/admin/roles/$ID

A curl example using an ``ID``

.. code-block:: bash

export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
export SERVER_URL=https://demo.dataverse.org
export SERVER_URL=http://localhost:8080
export ID=24

curl -H "X-Dataverse-key:$API_TOKEN" -X DELETE "$SERVER_URL/api/admin/roles/$ID"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import jakarta.persistence.TypedQuery;
//import jakarta.validation.constraints.NotNull;

/**
*
Expand All @@ -40,6 +39,9 @@ public class DataverseRoleServiceBean implements java.io.Serializable {

@EJB
RoleAssigneeServiceBean roleAssigneeService;

@EJB
DataverseServiceBean dataverseService;
@EJB
IndexServiceBean indexService;
@EJB
Expand All @@ -48,22 +50,23 @@ public class DataverseRoleServiceBean implements java.io.Serializable {
IndexAsync indexAsync;

public DataverseRole save(DataverseRole aRole) {
if (aRole.getId() == null) {
if (aRole.getId() == null) { // persist a new Role
em.persist(aRole);
/**
* @todo Why would getId be null? Should we call
* indexDefinitionPoint here too? A: it's null for new roles.
*/
return aRole;
} else {
DataverseRole merged = em.merge(aRole);
/**
* @todo update permissionModificationTime here.
*/
IndexResponse indexDefinitionPountResult = indexDefinitionPoint(merged.getOwner());
logger.info("aRole getId was not null. Indexing result: " + indexDefinitionPountResult);
return merged;
} else { // update an existing Role
aRole = em.merge(aRole);
}

DvObject owner = aRole.getOwner();
if(owner == null) { // Builtin Role
owner = dataverseService.findByAlias("root");
}

if(owner != null) { // owner may be null if a role is created before the root collection as in setup-all.sh
IndexResponse indexDefinitionPointResult = indexDefinitionPoint(owner);
logger.info("Indexing result: " + indexDefinitionPointResult);
}

return aRole;
}

public RoleAssignment save(RoleAssignment assignment) {
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/api/Admin.java
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,22 @@ public Response createNewBuiltinRole(RoleDTO roleDto) {
actionLogSvc.log(alr);
Copy link
Member

Choose a reason for hiding this comment

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

Above, at return ok(json(rolesSvc.save(roleDto.asRole()))); I'm getting the following error when trying to set up Dataverse for the first time:

{"status":"ERROR","message":"Exception thrown from bean: jakarta.ejb.EJBTransactionRolledbackException: Exception thrown from bean: java.lang.NullPointerException: Cannot invoke \"edu.harvard.iq.dataverse.DvObject.isInstanceofDataverse()\" because \"definitionPoint\" is null"}

Specifically, I'm doing this:

rm -rf docker-dev-volumes

mvn -Pct clean package docker:run

@luddaniel can you replicate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pdurbin I can replicate. Starting investigation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setup-all.sh creates roles before creating the root dataverse. Causing issue while creating a role

owner = dataverseService.findByAlias("root");
IndexResponse indexDefinitionPointResult = indexDefinitionPoint(owner);

Resulting in "definitionPoint" is null"

Fixed in my last commit.

}
}
@Path("roles/{id}")
@PUT
public Response updateBuiltinRole(RoleDTO roleDto, @PathParam("id") long roleId) {
ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.Admin, "updateBuiltInRole")
.setInfo(roleDto.getAlias() + ":" + roleDto.getDescription());
try {
DataverseRole role = roleDto.updateRoleFromDTO(rolesSvc.find(roleId));
return ok(json(rolesSvc.save(role)));
} catch (Exception e) {
alr.setActionResult(ActionLogRecord.Result.InternalError);
alr.setInfo(alr.getInfo() + "// " + e.getMessage());
return error(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage());
} finally {
actionLogSvc.log(alr);
}
}

@Path("roles")
@GET
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/edu/harvard/iq/dataverse/api/dto/RoleDTO.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ public void setPermissions(String[] permissions) {
this.permissions = permissions;
}

public DataverseRole asRole() {
DataverseRole r = new DataverseRole();
public DataverseRole updateRoleFromDTO(DataverseRole r) {
r.setAlias(alias);
r.setDescription(description);
r.setName(name);
r.clearPermissions();
if (permissions != null) {
if (permissions.length > 0) {
if (permissions[0].trim().toLowerCase().equals("all")) {
Expand All @@ -65,5 +65,9 @@ public DataverseRole asRole() {
}
return r;
}

public DataverseRole asRole() {
return updateRoleFromDTO(new DataverseRole());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
@RequiredPermissions(Permission.ManageDataversePermissions)
public class CreateRoleCommand extends AbstractCommand<DataverseRole> {

private final DataverseRole created;
private final DataverseRole role;
private final Dataverse dv;

public CreateRoleCommand(DataverseRole aRole, DataverseRequest aRequest, Dataverse anAffectedDataverse) {
super(aRequest, anAffectedDataverse);
created = aRole;
role = aRole;
dv = anAffectedDataverse;
}

Expand All @@ -41,16 +41,16 @@ public DataverseRole execute(CommandContext ctxt) throws CommandException {
//Test to see if the role already exists in DB
try {
DataverseRole testRole = ctxt.em().createNamedQuery("DataverseRole.findDataverseRoleByAlias", DataverseRole.class)
.setParameter("alias", created.getAlias())
.setParameter("alias", role.getAlias())
.getSingleResult();
if (!(testRole == null)) {
if (testRole != null && !testRole.getId().equals(role.getId())) {
throw new IllegalCommandException(BundleUtil.getStringFromBundle("permission.role.not.created.alias.already.exists"), this);
}
} catch (NoResultException nre) {
// we want no results because that meand we can create a role
// we want no results because that meant we can create a role
}
dv.addRole(created);
return ctxt.roles().save(created);
dv.addRole(role);
return ctxt.roles().save(role);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,12 @@ public IndexResponse indexPermissionsOnSelfAndChildren(long definitionPointId) {
* inheritance
*/
public IndexResponse indexPermissionsOnSelfAndChildren(DvObject definitionPoint) {

if (definitionPoint == null) {
logger.log(Level.WARNING, "Cannot perform indexPermissionsOnSelfAndChildren with a definitionPoint null");
return null;
}

List<DataFile> filesToReindexAsBatch = new ArrayList<>();
/**
* @todo Re-indexing the definition point itself seems to be necessary
Expand Down
Loading