Skip to content

Commit

Permalink
add missing argument not null assertions and update binary compatibil…
Browse files Browse the repository at this point in the history 
…ity check version to 1.1.0-M2

Signed-off-by: Florian Fendt <Florian.Fendt@bosch.io>
  • Loading branch information
@ffendt
ffendt committed Mar 16, 2020
1 parent 5217b27 commit 6e1996d
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 40 deletions.
2 changes: 1 addition & 1 deletion java/pom.xml
View file
Expand Up @@ -233,7 +233,7 @@
<!-- globally set version for checking binary compatibility against -->
<!-- whoever changes this to ${revision} or ${project.version} is responsible for API breakage caused by this! -->
<!-- in other words: never do that here! exclude the 'breakages' locally in the japicmp maven plugin if you intentionally break something -->
<binary-compatibility-check.version>1.0.0</binary-compatibility-check.version>
<binary-compatibility-check.version>1.1.0-M2</binary-compatibility-check.version>

This comment has been minimized.

Sign in to view

Copy link
@thjaeckle

thjaeckle Mar 16, 2020

Member

binary compatibility checks should only be done against real releases, not milestones
<!-- skip until first release: -->
<binary-compatibility-check.skip>false</binary-compatibility-check.skip>

Expand Down
71 changes: 44 additions & 27 deletions java/src/main/java/org/eclipse/ditto/client/internal/CommonManagementImpl.java
View file
Expand Up @@ -75,6 +75,9 @@
public abstract class CommonManagementImpl<T extends ThingHandle<F>, F extends FeatureHandle> implements
CommonManagement<T, F> {

private static final String ARGUMENT_THING_ID = "thingId";
private static final String ARGUMENT_THING = "thing";
private static final String ARGUMENT_INITIAL_POLICY = "initialPolicy";
private static final Logger LOGGER = LoggerFactory.getLogger(CommonManagementImpl.class);

private final TopicPath.Channel channel;
Expand Down Expand Up @@ -280,20 +283,21 @@ public CompletableFuture<Thing> create(final JsonObject jsonObject, final Option
}

@Override
public CompletableFuture<Thing> create(final Policy policy, final Option<?>... options) {
public CompletableFuture<Thing> create(final Policy initialPolicy, final Option<?>... options) {
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);
// as the backend adds the default namespace, we can here simply use the empty namespace.
final Thing thing = ThingsModelFactory.newThingBuilder()
.setId(ThingId.generateRandom())
.build();
return processCreate(thing, policy.toJson(), options);
return processCreate(thing, initialPolicy.toJson(), options);
}

@Override
public CompletableFuture<Thing> create(final ThingId thingId, final JsonObject initialPolicy,
final Option<?>... options) {
argumentNotNull(thingId);
argumentNotNull(thingId, ARGUMENT_THING_ID);
argumentNotEmpty(thingId);
argumentNotNull(initialPolicy);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

final Thing thing = ThingsModelFactory.newThingBuilder()
.setId(ThingId.of(thingId))
Expand All @@ -305,9 +309,9 @@ public CompletableFuture<Thing> create(final ThingId thingId, final JsonObject i
@Override
public CompletableFuture<Thing> create(final ThingId thingId, final Policy initialPolicy,
final Option<?>... options) {
argumentNotNull(thingId);
argumentNotNull(thingId, ARGUMENT_THING_ID);
argumentNotEmpty(thingId);
argumentNotNull(initialPolicy);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

final Thing thing = ThingsModelFactory.newThingBuilder()
.setId(ThingId.of(thingId))
Expand All @@ -316,37 +320,40 @@ public CompletableFuture<Thing> create(final ThingId thingId, final Policy initi
}

@Override
public CompletableFuture<Thing> create(final JsonObject jsonObject, final JsonObject initialPolicy,
public CompletableFuture<Thing> create(final JsonObject thing, final JsonObject initialPolicy,
final Option<?>... options) {
argumentNotNull(jsonObject);
argumentNotNull(initialPolicy);
argumentNotNull(thing, ARGUMENT_THING);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

final Thing thing = ThingsModelFactory.newThing(jsonObject);

return processCreate(thing, initialPolicy, options);
return processCreate(ThingsModelFactory.newThing(thing), initialPolicy, options);
}

@Override
public CompletableFuture<Thing> create(final JsonObject jsonObject, final Policy initialPolicy,
public CompletableFuture<Thing> create(final JsonObject thing, final Policy initialPolicy,
final Option<?>... options) {
argumentNotNull(jsonObject);
argumentNotNull(initialPolicy);

final Thing thing = ThingsModelFactory.newThing(jsonObject);
argumentNotNull(thing, ARGUMENT_THING);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

return processCreate(thing, initialPolicy.toJson(), options);
return processCreate(ThingsModelFactory.newThing(thing), initialPolicy.toJson(), options);
}

@Override
public CompletableFuture<Thing> create(final Thing thing, final JsonObject initialPolicy,
final Option<?>... options) {
argumentNotNull(thing, ARGUMENT_THING);
assertThatThingHasId(thing);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

return processCreate(thing, initialPolicy, options);
}


@Override
public CompletableFuture<Thing> create(final Thing thing, final Policy initialPolicy,
final Option<?>... options) {
argumentNotNull(thing, ARGUMENT_THING);
assertThatThingHasId(thing);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

return processCreate(thing, initialPolicy.toJson(), options);
}

Expand All @@ -370,6 +377,8 @@ private CompletableFuture<Thing> processCreate(final Thing thing, @Nullable fina

@Override
public CompletableFuture<Optional<Thing>> put(final Thing thing, final Option<?>... options) {
argumentNotNull(thing, ARGUMENT_THING);
assertThatThingHasId(thing);
return processPut(thing, null, options);
}

Expand All @@ -384,32 +393,40 @@ public CompletableFuture<Optional<Thing>> put(final JsonObject jsonObject, final
}

@Override
public CompletableFuture<Optional<Thing>> put(final JsonObject jsonObject, final JsonObject initialPolicy,
public CompletableFuture<Optional<Thing>> put(final JsonObject thing, final JsonObject initialPolicy,
final Option<?>... options) {
argumentNotNull(jsonObject);
argumentNotNull(thing, ARGUMENT_THING);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

final Thing thing = ThingsModelFactory.newThing(jsonObject);
return processPut(thing, initialPolicy, options);
return processPut(ThingsModelFactory.newThing(thing), initialPolicy, options);
}

@Override
public CompletableFuture<Optional<Thing>> put(final JsonObject jsonObject, final Policy initialPolicy,
public CompletableFuture<Optional<Thing>> put(final JsonObject thing, final Policy initialPolicy,
final Option<?>... options) {
argumentNotNull(jsonObject);
argumentNotNull(thing, ARGUMENT_THING);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

final Thing thing = ThingsModelFactory.newThing(jsonObject);
return processPut(thing, initialPolicy.toJson(), options);
return processPut(ThingsModelFactory.newThing(thing), initialPolicy.toJson(), options);
}

@Override
public CompletableFuture<Optional<Thing>> put(final Thing thing, final JsonObject initialPolicy,
final Option<?>... options) {
argumentNotNull(thing, ARGUMENT_THING);
assertThatThingHasId(thing);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

return processPut(thing, initialPolicy, options);
}

@Override
public CompletableFuture<Optional<Thing>> put(final Thing thing, final Policy initialPolicy,
final Option<?>... options) {
argumentNotNull(thing, ARGUMENT_THING);
assertThatThingHasId(thing);
argumentNotNull(initialPolicy, ARGUMENT_INITIAL_POLICY);

return processPut(thing, initialPolicy.toJson(), options);
}

Expand Down
26 changes: 16 additions & 10 deletions java/src/main/java/org/eclipse/ditto/client/management/CommonManagement.java
View file
Expand Up @@ -204,7 +204,8 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* org.eclipse.ditto.client.options.Options}.
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier.
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier, or if
* {@code initialPolicy} is {@code null}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
* @since 1.1.0
*/
Expand All @@ -220,7 +221,8 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* org.eclipse.ditto.client.options.Options}.
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @throws IllegalArgumentException if {@code thingId} is {@code null} or empty.
* @throws IllegalArgumentException if {@code thingId} is {@code null} or empty, or if {@code initialPolicy} is
* {@code null}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
* @since 1.1.0
*/
Expand All @@ -238,7 +240,7 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @throws IllegalArgumentException if {@code thing} is {@code null} or if it does not contain the field named
* {@code "thingId"}.
* {@code "thingId"}, or if {@code initialPolicy} is {@code null}.
* @throws org.eclipse.ditto.model.base.exceptions.DittoJsonException if {@code thing} cannot be parsed to a {@link
* Thing}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
Expand All @@ -255,7 +257,8 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* org.eclipse.ditto.client.options.Options}.
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier.
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier, or if
* {@code initialPolicy} is {@code null}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
* @since 1.1.0
*/
Expand All @@ -271,7 +274,8 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* org.eclipse.ditto.client.options.Options}.
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @throws IllegalArgumentException if {@code thingId} is {@code null} or empty.
* @throws IllegalArgumentException if {@code thingId} is {@code null} or empty, or if {@code initialPolicy} is
* {@code null}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
* @since 1.1.0
*/
Expand All @@ -289,7 +293,7 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* @return completable future providing the created Thing object or a specific {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed
* @throws IllegalArgumentException if {@code thing} is {@code null} or if it does not contain the field named
* {@code "thingId"}.
* {@code "thingId"}, or if {@code initialPolicy} is {@code null}.
* @throws org.eclipse.ditto.model.base.exceptions.DittoJsonException if {@code thing} cannot be parsed to a {@link
* Thing}.
* @throws org.eclipse.ditto.model.things.ThingIdInvalidException if the {@code thingId} was invalid.
Expand Down Expand Up @@ -344,7 +348,8 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* @return completable future providing an {@link Optional} containing the created Thing object, in case the Thing
* has been created, or an empty Optional, in case the Thing has been updated. Provides a {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed.
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier.
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier, or if
* {@code initialPolicy} is {@code null}.
* @since 1.1.0
*/
CompletableFuture<Optional<Thing>> put(Thing thing, JsonObject initialPolicy, Option<?>... options);
Expand All @@ -364,7 +369,7 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* has been created, or an empty Optional, in case the Thing has been updated. Provides a {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed.
* @throws IllegalArgumentException if {@code thing} is {@code null} or if it does not contain the field named
* {@code "thingId"}.
* {@code "thingId"}, or if {@code initialPolicy} is {@code null}.
* @throws org.eclipse.ditto.model.base.exceptions.DittoJsonException if {@code thing} cannot be parsed to a {@link
* Thing}.
* @since 1.1.0
Expand All @@ -383,7 +388,8 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* @return completable future providing an {@link Optional} containing the created Thing object, in case the Thing
* has been created, or an empty Optional, in case the Thing has been updated. Provides a {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed.
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier.
* @throws IllegalArgumentException if {@code thing} is {@code null} or has no identifier, or if
* {@code initialPolicy} is {@code null}.
* @since 1.1.0
*/
CompletableFuture<Optional<Thing>> put(Thing thing, Policy initialPolicy, Option<?>... options);
Expand All @@ -403,7 +409,7 @@ public interface CommonManagement<T extends ThingHandle, F extends FeatureHandle
* has been created, or an empty Optional, in case the Thing has been updated. Provides a {@link
* org.eclipse.ditto.model.base.exceptions.DittoRuntimeException} if the operation failed.
* @throws IllegalArgumentException if {@code thing} is {@code null} or if it does not contain the field named
* {@code "thingId"}.
* {@code "thingId"}, or if {@code initialPolicy} is {@code null}.
* @throws org.eclipse.ditto.model.base.exceptions.DittoJsonException if {@code thing} cannot be parsed to a {@link
* Thing}.
* @since 1.1.0
Expand Down
30 changes: 28 additions & 2 deletions java/src/test/java/org/eclipse/ditto/client/DittoClientThingTest.java
View file
Expand Up @@ -31,6 +31,7 @@
import org.eclipse.ditto.client.options.Options;
import org.eclipse.ditto.client.registration.DuplicateRegistrationIdException;
import org.eclipse.ditto.json.JsonFactory;
import org.eclipse.ditto.json.JsonObject;
import org.eclipse.ditto.json.JsonPointer;
import org.eclipse.ditto.model.base.auth.AuthorizationModelFactory;
import org.eclipse.ditto.model.base.auth.AuthorizationSubject;
Expand All @@ -39,6 +40,7 @@
import org.eclipse.ditto.model.messages.MessageDirection;
import org.eclipse.ditto.model.messages.MessageHeaders;
import org.eclipse.ditto.model.messages.MessagesModelFactory;
import org.eclipse.ditto.model.policies.Policy;
import org.eclipse.ditto.model.policies.PolicyId;
import org.eclipse.ditto.model.things.Feature;
import org.eclipse.ditto.model.things.Thing;
Expand Down Expand Up @@ -371,7 +373,7 @@ public void testCreateThingWithInlinePolicy() throws InterruptedException {
}

@Test
public void testCreateThingWithInitialJSONPolicy() throws InterruptedException {
public void testCreateThingWithInitialPolicyJson() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);

messaging.onSend(m -> {
Expand All @@ -388,6 +390,12 @@ public void testCreateThingWithInitialJSONPolicy() throws InterruptedException {
Assertions.assertThat(latch.await(TIMEOUT, TIME_UNIT)).isTrue();
}

@Test
public void testCreateThingWithInitialPolicyJsonNullable() {
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> client.twin().create(THING_ID, (JsonObject) null));
}

@Test
public void testCreateThingWithInitialPolicy() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);
Expand All @@ -406,6 +414,12 @@ public void testCreateThingWithInitialPolicy() throws InterruptedException {
Assertions.assertThat(latch.await(TIMEOUT, TIME_UNIT)).isTrue();
}

@Test
public void testCreateThingWithInitialPolicyNull() {
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> client.twin().create(THING_ID, (Policy) null));
}

@Test
public void testPutThingWithInlinePolicy() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);
Expand All @@ -425,7 +439,7 @@ public void testPutThingWithInlinePolicy() throws InterruptedException {
}

@Test
public void testPutThingWithInitialJSONPolicy() throws InterruptedException {
public void testPutThingWithInitialPolicyJson() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);

messaging.onSend(m -> {
Expand All @@ -442,6 +456,12 @@ public void testPutThingWithInitialJSONPolicy() throws InterruptedException {
Assertions.assertThat(latch.await(TIMEOUT, TIME_UNIT)).isTrue();
}

@Test
public void testPutThingWithInitialPolicyJsonNull() {
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> client.twin().put(THING, (JsonObject) null));
}

@Test
public void testPutThingWithInitialPolicy() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);
Expand All @@ -460,6 +480,12 @@ public void testPutThingWithInitialPolicy() throws InterruptedException {
Assertions.assertThat(latch.await(TIMEOUT, TIME_UNIT)).isTrue();
}

@Test
public void testPutThingWithInitialPolicyNull() {
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> client.twin().put(THING, (Policy) null));
}

@Test
public void testCreateThingWithOptionCopyPolicy() throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);
Expand Down

0 comments on commit 6e1996d

Please sign in to comment.