JCL-335: issue request builder & grant approval calls by timea-solid · Pull Request #422 · inrupt/solid-client-java

JCL-335: issue request builder & grant approval calls by timea-solid · Pull Request #422 · inrupt/solid-client-java

Conversation

The names are at the moment taken form the JS library:

Request issueAccessRequest
CompletionStage approveAccessRequest
We can of course rethink the names.

The issueAccessRequest has a toCompletableFuture().join() now, not sure.

Choose a reason for hiding this comment

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

Might be worth discussing this in more detail

		* @param request an AccessRequest
		* @return the next stage of completion containing the resulting credential
		*/
		public CompletionStage<AccessGrant> approveAccessRequest(final AccessRequest request) {

Choose a reason for hiding this comment

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

I would call this issueAccessGrant

Choose a reason for hiding this comment

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

issueAccessRequest name is to specify it is a request and not the actual issue call.
I call it better issueGrantRequest

		/**
		* An Access Grant issue request.
		*/
		public class AccessRequest {

Choose a reason for hiding this comment

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

I would have this extend or implement an AccessCredential type

		return this.data;
		}

		public AccessRequest(final Map<String, Object> data) {

Choose a reason for hiding this comment

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

I would make this more structured. A map is too general

I know we discussed removing the extra class but that would require me to pass Map<String, Object> around. Because of this, I try here to create more architecture.
Is it going in the right direction?

acoburn deleted the jcl-335-approveGrant branch

May 19, 2023 12:59