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

Command handler contract enforcement #930

Merged
merged 24 commits into from
Jan 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a1e25a8
Add tests that cover model verifier controlling for command handlers
serhii-lekariev Dec 26, 2018
76da15d
Enable model verifier to check command handler methods signatures
serhii-lekariev Dec 26, 2018
466abae
Merge branch 'master' into command-handler-contract-enforcement
serhii-lekariev Dec 26, 2018
4b9c0a3
Add warning suppression
serhii-lekariev Dec 26, 2018
8457761
Remove code with duplicate functionality
serhii-lekariev Jan 2, 2019
7c37e70
Enable warning logging in `MethodSignature`
serhii-lekariev Jan 2, 2019
52470d5
Fix formatting and remove redundant aggregate declaration
serhii-lekariev Jan 2, 2019
71b4e45
Fix method call alignment
serhii-lekariev Jan 2, 2019
fdc1a3a
Add tests that cover model verifier controlling for command handlers
serhii-lekariev Dec 26, 2018
f3325f9
Enable model verifier to check command handler methods signatures
serhii-lekariev Dec 26, 2018
f018b3a
Add warning suppression
serhii-lekariev Dec 26, 2018
5c64961
Remove code with duplicate functionality
serhii-lekariev Jan 2, 2019
9ead09c
Enable warning logging in `MethodSignature`
serhii-lekariev Jan 2, 2019
368da1d
Fix formatting and remove redundant aggregate declaration
serhii-lekariev Jan 2, 2019
775ecbe
Fix method call alignment
serhii-lekariev Jan 2, 2019
823ea57
Merge remote-tracking branch 'origin/command-handler-contract-enforce…
Jan 4, 2019
779fe62
Test producing of a warning upon finding a private command handler
serhii-lekariev Jan 4, 2019
55e4d02
Add static import
serhii-lekariev Jan 4, 2019
2a1ce90
Merge remote-tracking branch 'origin/master' into command-handler-con…
Jan 4, 2019
1910334
Fix event subscribers with wrong access modifiers
serhii-lekariev Jan 4, 2019
f8d16be
Add a missing static import
serhii-lekariev Jan 4, 2019
1a1b2fe
Fix formatting
serhii-lekariev Jan 4, 2019
df1272a
Get rid of `getSeverity()` in favour of `is...` for both warn and error
serhii-lekariev Jan 4, 2019
82e4d31
Revert unwanted changes
serhii-lekariev Jan 4, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ private URLClassLoader createClassLoader(Project project) {
}
}


private static Collection<JavaCompile> allJavaCompile(Project project) {
Collection<JavaCompile> tasks = newArrayList();
ProjectHierarchy.applyToAll(project.getRootProject(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@
package io.spine.model.verify;

import com.google.common.io.Files;
import io.spine.logging.Logging;
import io.spine.model.CommandHandlers;
import io.spine.model.verify.ModelVerifier.GetDestinationDir;
import io.spine.model.verify.given.DuplicateCommandHandler;
import io.spine.model.verify.given.EditAggregate;
import io.spine.model.verify.given.InvalidDeleteAggregate;
import io.spine.model.verify.given.InvalidEnhanceAggregate;
import io.spine.model.verify.given.InvalidRestoreAggregate;
import io.spine.model.verify.given.RenameProcMan;
import io.spine.model.verify.given.UploadCommandHandler;
import io.spine.server.model.DuplicateCommandHandlerError;
import io.spine.server.model.declare.MethodSignature;
import io.spine.server.model.declare.SignatureMismatchException;
import io.spine.testing.logging.MuteLogging;
import org.gradle.api.Project;
import org.gradle.api.initialization.dsl.ScriptHandler;
Expand All @@ -37,13 +43,23 @@
import org.gradle.internal.impldep.com.google.common.collect.Iterators;
import org.gradle.testfixtures.ProjectBuilder;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.slf4j.event.Level;
import org.slf4j.event.SubstituteLoggingEvent;
import org.slf4j.helpers.SubstituteLogger;

import java.io.File;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayDeque;
import java.util.Queue;
import java.util.function.Function;
import java.util.stream.Stream;

import static io.spine.tools.gradle.TaskName.COMPILE_JAVA;
import static java.util.Collections.emptySet;
Expand Down Expand Up @@ -105,6 +121,24 @@ void verifyModel() {
verifier.verify(spineModel);
}

@ParameterizedTest
@DisplayName("throw when attempting to verify a model that declares an invalid command handler")
@MethodSource("getBadHandlers")
void throwOnSignatureMismatch(String badHandlerName) {
ModelVerifier verifier = new ModelVerifier(project);
CommandHandlers model = CommandHandlers
.newBuilder()
.addCommandHandlingTypes(badHandlerName)
.build();
assertThrows(SignatureMismatchException.class, () -> verifier.verify(model));
}

private static Stream<Arguments> getBadHandlers() {
return Stream.of(
Arguments.of(InvalidDeleteAggregate.class.getName()),
Arguments.of(InvalidEnhanceAggregate.class.getName()));
}

@Test
@DisplayName("fail on duplicate command handlers")
void failOnDuplicateHandlers() {
Expand All @@ -120,6 +154,30 @@ void failOnDuplicateHandlers() {
assertThrows(DuplicateCommandHandlerError.class, () -> verifier.verify(spineModel));
}

@Disabled("until Gradle wrapper version is updated to at least 5.0")
///TODO:2018-01-04:serhii.lekariev: enable when https://github.com/SpineEventEngine/core-java/issues/932 is resolved
@Test
@DisplayName("produce a warning on private command handling methods")
void warnOnPrivateHandlers(){
ModelVerifier verifier = new ModelVerifier(project);
Queue<SubstituteLoggingEvent> loggedMessages = redirectLogging();
dmdashenkov marked this conversation as resolved.
Show resolved Hide resolved
CommandHandlers model = CommandHandlers
.newBuilder()
.addCommandHandlingTypes(InvalidRestoreAggregate.class.getName())
.build();
verifier.verify(model);
assertEquals(1, loggedMessages.size());
SubstituteLoggingEvent event = loggedMessages.poll();
assertEquals(event.getLevel(), Level.WARN);
}

/** Redirects logging produced by model verifier to a {@code Queue} that is returned. */
private static Queue<SubstituteLoggingEvent> redirectLogging() {
Queue<SubstituteLoggingEvent> loggedMessages = new ArrayDeque<>();
Logging.redirect((SubstituteLogger) Logging.get(MethodSignature.class), loggedMessages);
return loggedMessages;
}

@Test
@MuteLogging
@DisplayName("ignore invalid class names")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
///*
// * Copyright 2018, TeamDev. All rights reserved.
// *
// * Redistribution and use in source and/or binary forms, with or without
// * modification, must retain the above copyright notice and the following
// * disclaimer.
// *
// * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// */
//
package io.spine.model.verify.given;

import io.spine.server.aggregate.Aggregate;
import io.spine.server.command.Assign;
import io.spine.test.model.verify.command.DeletePhoto;
import io.spine.test.model.verify.event.PhotoDeleted;
import io.spine.test.model.verify.given.EditState;
import io.spine.test.model.verify.given.EditStateVBuilder;

/**
* This aggregate declares a command handling method that breaks the contract imposed by
* {@link Assign}, by accepting a first parameter of type that cannot be derived from
* {@link io.spine.base.CommandMessage}.
*/
public class InvalidDeleteAggregate extends Aggregate<String, EditState, EditStateVBuilder> {

protected InvalidDeleteAggregate(String id) {
super(id);
}

@Assign
PhotoDeleted handle(String unnecessaryString, DeletePhoto delete) {
return PhotoDeleted
.newBuilder()
.setTitle(delete.getTitle())
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2018, TeamDev. All rights reserved.
*
* Redistribution and use in source and/or binary forms, with or without
* modification, must retain the above copyright notice and the following
* disclaimer.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

package io.spine.model.verify.given;

import io.spine.server.aggregate.Aggregate;
import io.spine.server.command.Assign;
import io.spine.test.model.verify.command.EnhancePhoto;
import io.spine.test.model.verify.given.EditState;
import io.spine.test.model.verify.given.EditStateVBuilder;

/**
* This aggregate declares a command handling method that breaks the contract imposed by
* {@link Assign}, by having a return value that cannot be derived from
* {@link io.spine.base.EventMessage}.
*/
public class InvalidEnhanceAggregate extends Aggregate<String, EditState, EditStateVBuilder> {

protected InvalidEnhanceAggregate(String id) {
super(id);
}

@Assign
String handle(EnhancePhoto delete) {
return delete.getTitle();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2018, TeamDev. All rights reserved.
*
* Redistribution and use in source and/or binary forms, with or without
* modification, must retain the above copyright notice and the following
* disclaimer.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

package io.spine.model.verify.given;

import io.spine.server.aggregate.Aggregate;
import io.spine.server.command.Assign;
import io.spine.test.model.verify.command.RestorePhoto;
import io.spine.test.model.verify.event.PhotoRestored;
import io.spine.test.model.verify.given.EditState;
import io.spine.test.model.verify.given.EditStateVBuilder;

/**
* This aggregate declares a command handling method that breaks the contract imposed by
* {@link Assign}, by having a {@code private} access modifier.
*
* <p>This should result in a warning.
*/
public class InvalidRestoreAggregate extends Aggregate<String, EditState, EditStateVBuilder> {

protected InvalidRestoreAggregate(String id) {
super(id);
}

@Assign
private PhotoRestored handle(RestorePhoto restore){
return PhotoRestored.newBuilder()
.setTitle(restore.getTitle())
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,15 @@ message EditPhoto {
message ChangeTitle {
string new_title = 1;
}

message DeletePhoto {
string title = 1;
}

message RestorePhoto {
string title = 1;
}

message EnhancePhoto {
string title = 1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,15 @@ message PhotoEdited {
message TitleChanged {
string new_title = 1;
}

message PhotoDeleted {
string title = 1;
}

message PhotoEnhanced {
string title = 1;
}

message PhotoRestored {
string title = 1;
}
4 changes: 2 additions & 2 deletions model/model-verifier/src/test/resources/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
buildscript {

// Apply the script created by `GradleProject.createTestEnvExt()` which defines
// `enclosingRootDir` variable that we use below.
// `enclosingRootDir` variable that we use below.
apply from: "$rootDir/testEnv.gradle"

// Apply shared dependencies.
apply from: "$enclosingRootDir/config/gradle/dependencies.gradle"

// Applying from `version.gradle` inside the `buildscript` section to reuse the properties.
//
// As long as `buildscript` section is always evaluated first, we need to apply
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@

import com.google.common.collect.ImmutableSet;
import io.spine.core.MessageEnvelope;
import io.spine.logging.Logging;
import io.spine.server.model.HandlerMethod;

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;

import static com.google.common.base.Preconditions.checkNotNull;
import static io.spine.server.model.declare.MethodParams.findMatching;
import static io.spine.server.model.declare.SignatureMismatch.Severity.ERROR;
import static java.util.stream.Collectors.toList;

/**
Expand All @@ -43,16 +44,12 @@
*
* <p>By extending this base class, descendants define the number of requirements:
* <ul>
* <li>{@linkplain #MethodSignature(Class) the method annotation},</li>
*
* <li>{@linkplain #getParamSpecs() the specification of method parameters},</li>
*
* <li>{@linkplain #getAllowedModifiers() the set of allowed access modifiers},</li>
*
* <li>{@linkplain #getValidReturnTypes() the set of valid return types},</li>
*
* <li>{@linkplain #getAllowedExceptions() the set of allowed exceptions}, that the method
* declares to throw (empty by default),</li>
* <li>{@linkplain #MethodSignature(Class) the method annotation},
* <li>{@linkplain #getParamSpecs() the specification of method parameters},
* <li>{@linkplain #getAllowedModifiers() the set of allowed access modifiers},
* <li>{@linkplain #getValidReturnTypes() the set of valid return types},
* <li>{@linkplain #getAllowedExceptions() the set of allowed exceptions}, that the method
* declares to throw (empty by default),
* </ul>
*
* @param <H> the type of the handler method
Expand All @@ -61,7 +58,7 @@
* @author Alex Tymchenko
*/
public abstract class MethodSignature<H extends HandlerMethod<?, ?, E, ?>,
E extends MessageEnvelope<?, ?, ?>> {
E extends MessageEnvelope<?, ?, ?>> implements Logging {

private final Class<? extends Annotation> annotation;

Expand Down Expand Up @@ -118,10 +115,18 @@ public boolean matches(Method method) throws SignatureMismatchException {
}
Collection<SignatureMismatch> mismatches = match(method);
boolean hasErrors = mismatches.stream()
.anyMatch(mismatch -> ERROR == mismatch.getSeverity());
.anyMatch(SignatureMismatch::isError);
List<SignatureMismatch> warnings = mismatches.stream()
.filter(SignatureMismatch::isWarning)
.collect(toList());
if (hasErrors) {
throw new SignatureMismatchException(mismatches);
}
if (!warnings.isEmpty()) {
warnings.stream()
.map(SignatureMismatch::toString)
.forEach(this::_warn);
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,14 @@ private SignatureMismatch(MatchCriterion criterion, Object[] values) {
message = criterion.formatMsg(values);
}

/**
* Returns the severity of the mismatch.
*/
Severity getSeverity() {
return severity;
/** Returns whether this mismatch is of {@code ERROR} severity. */
boolean isError() {
return severity == Severity.ERROR;
}

/** Returns whether this mismatch is of {@code WARN} severity. */
boolean isWarning() {
return severity == Severity.WARN;
}

/**
Expand Down
Loading