Navigation Menu

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

Add basic level policy support to Ditto Java Client #46

Merged
merged 43 commits into from Mar 17, 2020

Conversation

VadimGue
Copy link
Contributor

Basic Policy management over Ditto Java Client. Includes create, read, update and delete a Policy.
Also when creating a Thing inline Policy as well as copy Policy is supported. (See: https://www.eclipse.org/ditto/protocol-examples-creatething.html#alternative-creatething-commands)

VadimGue and others added 30 commits November 28, 2019 15:00
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Dominik Guggemos <dominik.guggemos@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
…om/stash/scm/ditto/ditto-clients into feature/policy-protocol

Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
…atibility

Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
…Thing, as an already existing Thing can never get a new initial Policy

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
…ied Policy

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
…gMessageFactory

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
…ions instead of implementing own helper methods

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
…hingCommand

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
…om/stash/scm/ditto/ditto-clients into feature/policy-protocol

Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
…egrationTest.

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
ffendt and others added 7 commits February 24, 2020 14:52
…d classes.

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
…ponse forwarding

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Signed-off-by: Dominik Guggemos <dominik.guggemos@bosch-si.com>
Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
* @since 1.1.0
*/
CompletableFuture<Thing> create(JsonObject thing, JsonObject initialPolicy, Option<?>... options);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason why the create and put methods with initialPolicy all only provide JsonObject instead of a Policy object?
That would be way more convenient to use IMO. Either in addition to the JsonObject or maybe even replacing it.

Also some have the initialPolicy param @Nullable, others don't. Is that intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Providing create and put methods with initialPolicy using a Policy object is a good point. I include this in addition to the existing possibilities.

The methods with the @Nullable once in put and in the create method are called by all other put and create methods. This is where the actual processing takes place.
Maybe it would be better to introduce a private method which does the processing? This would remove the @Nullable from the two public put and create methods.

Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Please fix some additional findings on the latest commit.

Also: the ECA is failing again because of commits without correct sign-offs.

* Thing}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
*/
CompletableFuture<Thing> create(JsonObject thing, Option<?>... options);
Copy link
Member

Choose a reason for hiding this comment

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

@since 1.1.0 is missing

* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
*/
CompletableFuture<Thing> create(Policy initialPolicy, Option<?>... options);
Copy link
Member

Choose a reason for hiding this comment

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

@since 1.1.0 is missing

*
* @param options options to be applied configuring behaviour of this method, see {@link
* org.eclipse.ditto.client.options.Options}.
* @param initialPolicy a custom policy to use for the Thing instead of the default Policy.
Copy link
Member

Choose a reason for hiding this comment

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

the intialPolicy param should be documented before the options param - in order of their appearance in the signature

CompletableFuture<Thing> create(JsonObject thing, Option<?>... options);

/**
* Creates an empty {@link Thing} with an auto-generated identifier.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add "with an auto-generated identifier" and an initial Policy?

* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
* @since 1.1.0
*/
CompletableFuture<Thing> create(Thing thing, final Policy initialPolicy, Option<?>... options);
Copy link
Member

Choose a reason for hiding this comment

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

In interface signatures we don't use final, please remove from the Policy param

* @throws org.eclipse.ditto.model.base.exceptions.DittoJsonException if {@code thing} cannot be parsed to a {@link
* Thing}.
*/
CompletableFuture<Optional<Thing>> put(JsonObject thing, Option<?>... options);
Copy link
Member

Choose a reason for hiding this comment

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

@since 1.1.0 is missing

Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @since 1.0.0
Copy link
Member

Choose a reason for hiding this comment

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

since 1.0.0? Don't think so ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was a typo combined with copy + paste... I will fix that

@@ -326,6 +328,7 @@
* {@code "thingId"}.
* @throws org.eclipse.ditto.model.base.exceptions.DittoJsonException if {@code thing} cannot be parsed to a {@link
* Thing}.
* @since 1.0.0
Copy link
Member

@thjaeckle thjaeckle Mar 5, 2020

Choose a reason for hiding this comment

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

was this method already in 1.0.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The interface method put(JsonObject thing, Option<?>... options) was already there from the beginning. So imo @since 1.0.0 must be right

Signed-off-by: Vadim Guenther <vadim.guenther@bosch-si.com>
@ffendt
Copy link
Contributor

ffendt commented Mar 5, 2020

I fixed the ECA check but the build will keep failing until we have a Ditto version that provides TopicPath.Channel.NONE

…ity check version to 1.1.0-M2

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
… check should only be done against real releases

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
…ree"

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
@ffendt ffendt merged commit 99d5a7a into eclipse-ditto:master Mar 17, 2020
@ffendt ffendt deleted the feature/policy-protocol branch March 17, 2020 10:06
@thjaeckle thjaeckle added this to the 1.1.0 milestone Apr 27, 2020
@thjaeckle thjaeckle added the java label Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants