Статьи

Студент проекта: упрощение кода с помощью АОП

Январь 21, 2019

Это часть проекта Студент .

Многие люди твердо убеждены, что методы должны уместиться в вашем окне редактора (скажем, 20 строк), а некоторые люди считают, что методы должны быть даже меньше, чем это. Идея состоит в том, что метод должен делать одно и только одно. Если он делает больше, чем это, он должен быть разбит на несколько методов, и «одна вещь» старого метода — это координация новых методов.

Это не означает разделение метода после произвольного числа строк. Иногда методы естественно больше. Это всегда хороший вопрос.

Итак, как вы узнаете код, который делает больше, чем одну вещь? Хороший пробный камень — если код дублируется несколькими способами. Каноническим примером является управление транзакциями в персистентных классах. Это нужно каждому классу персистентности, и код всегда выглядит одинаково.

Другой пример — обработчик необработанных исключений в моих классах ресурсов. Каждый метод, обращающийся к REST, должен иметь дело с этим, и код всегда выглядит одинаково.

Это теория. На практике код может быть некрасивым и приносить скромные выгоды. К счастью, есть решение: Аспектно-ориентированное программирование (AOP). Это позволяет нам прозрачно создавать код до или после вызова нашего метода. Это часто позволяет нам значительно упростить наши методы.

Проектные решения

AspectJ — я использую AspectJ через Spring инъекцию.

Ограничения

Выражения AspectJ pointcut относительно просты с методами CRUD. Это может быть неверно, так как добавлена ​​более сложная функциональность.

Необработанные исключения в методах ресурсов

Нашей первой заботой являются необработанные исключения в ресурсных методах. Джерси в любом случае вернет сообщение SERVER INTERNAL ERROR (500), но, вероятно, оно будет содержать трассировку стека и другой контент, который мы не хотим, чтобы злоумышленник знал. Мы можем контролировать то, что оно включает, если отправим это сами. Мы могли бы добавить блок «catch» во всех наших методах, но это шаблон, который мы можем переместить в метод AOP. Это сделает все наши методы Resource намного тоньше и удобнее для чтения.

Этот класс также проверяет исключения «Объект не найден». Это было бы легко обрабатывать в отдельных классах ресурсов, но это запутывает код. Размещение здесь обработчика исключений позволяет нашим методам сосредоточиться на удачном пути и гарантирует последовательность в ответе.

Этот класс имеет две оптимизации. Во-первых, он явно проверяет исключение UnitTestException и в этом случае пропускает подробное ведение журнала. Одна из моих самых больших любимых мозолей — тесты, которые заполняют журналы следами стека, когда все работает, как ожидалось. Это делает невозможным просмотр логов для очевидных проблем. Это единственное изменение может значительно облегчить поиск проблем.

Во-вторых, он использует регистратор, связанный с целевым классом, например, CourseResource , а не с классом AOP. Помимо большей ясности, это позволяет нам выборочно изменять уровень ведения журнала для одного ресурса вместо всех.

Другой трюк заключается в вызове ExceptionService в нашем обработчике. Это сервис, который может делать что-то полезное с исключениями, например, он может создавать или обновлять билет Jira. Это не было реализовано, поэтому я просто добавил комментарий, чтобы показать, куда это идет.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
@Aspect
@Component
public class UnexpectedResourceExceptionHandler {
    @Around("target(com.invariantproperties.sandbox.student.webservice.server.rest.AbstractResource)")
    public Object checkForUnhandledException(ProceedingJoinPoint pjp) throws Throwable {
        Object results = null;
        Logger log = Logger.getLogger(pjp.getSignature().getClass());
 
        try {
            results = pjp.proceed(pjp.getArgs());
        } catch (ObjectNotFoundException e) {
            // this is safe to log since we know that we've passed filtering.
            String args = Arrays.toString(pjp.getArgs());
            results = Response.status(Status.NOT_FOUND).entity("object not found: " + args).build();
            if (log.isDebugEnabled()) {
                log.debug("object not found: " + args);
            }
        } catch (Exception e) {
            // find the method we called. We can't cache this since the method
            // may be overloaded
            Method method = findMethod(pjp);
            if ((method != null) && Response.class.isAssignableFrom(method.getReturnType())) {
                // if the method returns a response we can return a 500 message.
                if (!(e instanceof UnitTestException)) {
                    if (log.isInfoEnabled()) {
                        log.info(
                                String.format("%s(): unhandled exception: %s", pjp.getSignature().getName(),
                                        e.getMessage()), e);
                    }
                } else if (log.isTraceEnabled()) {
                    log.info("unit test exception: " + e.getMessage());
                }
                results = Response.status(Status.INTERNAL_SERVER_ERROR).build();
            } else {
                // DO NOT LOG THE EXCEPTION. That just clutters the log - let
                // the final handler log it.
                throw e;
            }
        }
 
        return results;
    }
 
    /**
     * Find method called via reflection.
     */
    Method findMethod(ProceedingJoinPoint pjp) {
        Class[] argtypes = new Class[pjp.getArgs().length];
        for (int i = 0; i < argtypes.length; i++) {
            argtypes[i] = pjp.getArgs()[i].getClass();
        }
 
        Method method = null;
 
        try {
            // @SuppressWarnings("unchecked")
            method = pjp.getSignature().getDeclaringType().getMethod(pjp.getSignature().getName(), argtypes);
        } catch (Exception e) {
            Logger.getLogger(UnexpectedResourceExceptionHandler.class).info(
                    String.format("could not find method for %s.%s", pjp.getSignature().getDeclaringType().getName(),
                            pjp.getSignature().getName()));
        }
 
        return method;
    }
}

Проверка значений REST Post

Наши методы Resource также имеют много стандартного кода для проверки параметров REST. Являются ли они ненулевыми, правильно ли сформированы адреса электронной почты и т. Д. Опять же, легко перенести большую часть этого кода в метод AOP и упростить метод Resource.

Мы начнем с определения интерфейса, который указывает, что объект передачи REST может быть проверен. Эта первая версия дает нам простой или большой палец вверх, улучшенная версия даст нам способ рассказать клиенту, какие конкретные проблемы.

1
2
3
4
public interface Validatable {
 
    boolean validate();
}

Теперь мы расширим наши более ранние объекты передачи REST, чтобы добавить метод проверки.

Две заметки. Во-первых, имя и адрес электронной почты принимают буквы Unicode, а не только стандартные буквы ASCII. Это важно, поскольку наш мир становится интернационализированным.

Во-вторых, я добавил метод toString (), но это небезопасно, поскольку он использует неанизированные значения. Я займусь санитарией позже.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
@XmlRootElement
public class NameAndEmailAddressRTO implements Validatable {
 
    // names must be alphabetic, an apostrophe, a dash or a space. (Anne-Marie,
    // O'Brien). This pattern should accept non-Latin characters.
    // digits and colon are added to aid testing. Unlikely but possible in real
    // names.
    private static final Pattern NAME_PATTERN = Pattern.compile("^[\\p{L}\\p{Digit}' :-]+$");
 
    // email address must be well-formed. This pattern should accept non-Latin
    // characters.
    private static final Pattern EMAIL_PATTERN = Pattern.compile("^[^@]+@([\\p{L}\\p{Digit}-]+\\.)?[\\p{L}]+");
 
    private String name;
    private String emailAddress;
    private String testUuid;
 
    public String getName() {
        return name;
    }
 
    public void setName(String name) {
        this.name = name;
    }
 
    public String getEmailAddress() {
        return emailAddress;
    }
 
    public void setEmailAddress(String emailAddress) {
        this.emailAddress = emailAddress;
    }
 
    public String getTestUuid() {
        return testUuid;
    }
 
    public void setTestUuid(String testUuid) {
        this.testUuid = testUuid;
    }
 
    /**
     * Validate values.
     */
    @Override
    public boolean validate() {
        if ((name == null) || !NAME_PATTERN.matcher(name).matches()) {
            return false;
        }
 
        if ((emailAddress == null) || !EMAIL_PATTERN.matcher(emailAddress).matches()) {
            return false;
        }
 
        if ((testUuid != null) && !StudentUtil.isPossibleUuid(testUuid)) {
            return false;
        }
 
        return true;
    }
 
    @Override
    public String toString() {
        // FIXME: this is unsafe!
        return String.format("NameAndEmailAddress('%s', '%s', %s)", name, emailAddress, testUuid);
    }
}

Мы вносим аналогичные изменения в другие объекты передачи REST.

Теперь мы можем написать наши методы AOP для проверки параметров наших операций CRUD. Как и прежде, журналы записываются с использованием регистратора, связанного с ресурсом, вместо класса AOP.

Эти методы также регистрируют запись методов Resource. Опять же, это шаблон, и здесь это упрощает методы Resource. Было бы тривиально также регистрировать выход метода и прошедшее время, но в этом случае мы должны использовать класс AOP регистратора акций.

001
002
003
004
005
006
007
008
009
010
011
012
013
014
015
016
017
018
019
020
021
022
023
024
025
026
027
028
029
030
031
032
033
034
035
036
037
038
039
040
041
042
043
044
045
046
047
048
049
050
051
052
053
054
055
056
057
058
059
060
061
062
063
064
065
066
067
068
069
070
071
072
073
074
075
076
077
078
079
080
081
082
083
084
085
086
087
088
089
090
091
092
093
094
095
096
097
098
099
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
@Aspect
@Component
public class CheckPostValues {
 
    /**
     * Check post values on create method.
     *
     * @param pjp
     * @return
     * @throws Throwable
     */
    @Around("target(com.invariantproperties.sandbox.student.webservice.server.rest.AbstractResource) && args(validatable,..)")
    public Object checkParametersCreate(ProceedingJoinPoint pjp, Validatable rto) throws Throwable {
        final Logger log = Logger.getLogger(pjp.getSignature().getDeclaringType());
        final String name = pjp.getSignature().getName();
        Object results = null;
 
        if (rto.validate()) {
            // this should be safe since parameters have been validated.
            if (log.isDebugEnabled()) {
                log.debug(String.format("%s(%s): entry", name, Arrays.toString(pjp.getArgs())));
            }
            results = pjp.proceed(pjp.getArgs());
        } else {
            // FIXME: this is unsafe
            if (log.isInfoEnabled()) {
                log.info(String.format("%s(%s): bad arguments", name, Arrays.toString(pjp.getArgs())));
            }
            // TODO: tell caller what the problems were
            results = Response.status(Status.BAD_REQUEST).build();
        }
 
        return results;
    }
 
    /**
     * Check post values on update method.
     *
     * @param pjp
     * @return
     * @throws Throwable
     */
    @Around("target(com.invariantproperties.sandbox.student.webservice.server.rest.AbstractResource) && args(uuid,validatable,..)")
    public Object checkParametersUpdate(ProceedingJoinPoint pjp, String uuid, Validatable rto) throws Throwable {
        final Logger log = Logger.getLogger(pjp.getSignature().getDeclaringType());
        final String name = pjp.getSignature().getName();
        Object results = null;
 
        if (!StudentUtil.isPossibleUuid(uuid)) {
            // this is a possible attack.
            if (log.isInfoEnabled()) {
                log.info(String.format("%s(): uuid", name));
            }
            results = Response.status(Status.BAD_REQUEST).build();
        } else if (rto.validate()) {
            // this should be safe since parameters have been validated.
            if (log.isDebugEnabled()) {
                log.debug(String.format("%s(%s): entry", name, Arrays.toString(pjp.getArgs())));
            }
            results = pjp.proceed(pjp.getArgs());
        } else {
            // FIXME: this is unsafe
            if (log.isInfoEnabled()) {
                log.info(String.format("%s(%s): bad arguments", name, Arrays.toString(pjp.getArgs())));
            }
            // TODO: tell caller what the problems were
            results = Response.status(Status.BAD_REQUEST).build();
        }
 
        return results;
    }
 
    /**
     * Check post values on delete method. This is actually a no-op but it
     * allows us to log method entry.
     *
     * @param pjp
     * @return
     * @throws Throwable
     */
    @Around("target(com.invariantproperties.sandbox.student.webservice.server.rest.AbstractResource) && args(uuid,version) && execution(* *.delete*(..))")
    public Object checkParametersDelete(ProceedingJoinPoint pjp, String uuid, Integer version) throws Throwable {
        final Logger log = Logger.getLogger(pjp.getSignature().getDeclaringType());
        final String name = pjp.getSignature().getName();
        Object results = null;
 
        if (!StudentUtil.isPossibleUuid(uuid)) {
            // this is a possible attack.
            if (log.isInfoEnabled()) {
                log.info(String.format("%s(): uuid", name));
            }
            results = Response.status(Status.BAD_REQUEST).build();
        } else {
            // this should be safe since parameters have been validated.
            if (log.isDebugEnabled()) {
                log.debug(String.format("%s(%s): entry", name, Arrays.toString(pjp.getArgs())));
            }
            results = pjp.proceed(pjp.getArgs());
        }
 
        return results;
    }
 
    /**
     * Check post values on find methods. This is actually a no-op but it allows
     * us to log method entry.
     *
     * @param pjp
     * @return
     * @throws Throwable
     */
    @Around("target(com.invariantproperties.sandbox.student.webservice.server.rest.AbstractResource) && execution(* *.find*(..))")
    public Object checkParametersFind(ProceedingJoinPoint pjp) throws Throwable {
        final Logger log = Logger.getLogger(pjp.getSignature().getDeclaringType());
 
        if (log.isDebugEnabled()) {
            log.debug(String.format("%s(%s): entry", pjp.getSignature().getName(), Arrays.toString(pjp.getArgs())));
        }
        final Object results = pjp.proceed(pjp.getArgs());
 
        return results;
    }
}

Обновленная конфигурация Spring

Мы должны сказать Spring, чтобы искать классы АОП. Это однострочное изменение в нашем файле конфигурации.

Обновленный ресурс

Теперь мы можем упростить наши классы ресурсов. Несколько методов были сведены к одному счастливому пути.

001
002
003
004
005
006
007
008
009
010
011
012
013
014
015
016
017
018
019
020
021
022
023
024
025
026
027
028
029
030
031
032
033
034
035
036
037
038
039
040
041
042
043
044
045
046
047
048
049
050
051
052
053
054
055
056
057
058
059
060
061
062
063
064
065
066
067
068
069
070
071
072
073
074
075
076
077
078
079
080
081
082
083
084
085
086
087
088
089
090
091
092
093
094
095
096
097
098
099
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
@Service
@Path("/course")
public class CourseResource extends AbstractResource {
    private static final Logger LOG = Logger.getLogger(CourseResource.class);
    private static final Course[] EMPTY_COURSE_ARRAY = new Course[0];
 
    @Resource
    private CourseFinderService finder;
 
    @Resource
    private CourseManagerService manager;
 
    @Resource
    private TestRunService testRunService;
 
    /**
     * Default constructor.
     */
    public CourseResource() {
 
    }
 
    /**
     * Set values used in unit tests. (Required due to AOP)
     *
     * @param finder
     * @param manager
     * @param testService
     */
    void setServices(CourseFinderService finder, CourseManagerService manager, TestRunService testRunService) {
        this.finder = finder;
        this.manager = manager;
        this.testRunService = testRunService;
    }
 
    /**
     * Get all Courses.
     *
     * @return
     */
    @GET
    @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    public Response findAllCourses() {
        final List courses = finder.findAllCourses();
 
        final List results = new ArrayList(courses.size());
        for (Course course : courses) {
            results.add(scrubCourse(course));
        }
 
        final Response response = Response.ok(results.toArray(EMPTY_COURSE_ARRAY)).build();
 
        return response;
    }
 
    /**
     * Create a Course.
     *
     * FIXME: what about uniqueness violations?
     *
     * @param req
     * @return
     */
    @POST
    @Consumes({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    public Response createCourse(CourseInfo req) {
        final String code = req.getCode();
        final String name = req.getName();
 
        Response response = null;
        Course course = null;
 
        if (req.getTestUuid() != null) {
            TestRun testRun = testRunService.findTestRunByUuid(req.getTestUuid());
            if (testRun != null) {
                course = manager.createCourseForTesting(code, name, req.getSummary(), req.getDescription(),
                        req.getCreditHours(), testRun);
            } else {
                response = Response.status(Status.BAD_REQUEST).entity("unknown test UUID").build();
            }
        } else {
            course = manager.createCourse(code, name, req.getSummary(), req.getDescription(), req.getCreditHours());
        }
        if (course == null) {
            response = Response.status(Status.INTERNAL_SERVER_ERROR).build();
        } else {
            response = Response.created(URI.create(course.getUuid())).entity(scrubCourse(course)).build();
        }
 
        return response;
    }
 
    /**
     * Get a specific Course.
     *
     * @param uuid
     * @return
     */
    @Path("/{courseId}")
    @GET
    @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    public Response getCourse(@PathParam("courseId") String id) {
 
        // 'object not found' handled by AOP
        Course course = finder.findCourseByUuid(id);
        final Response response = Response.ok(scrubCourse(course)).build();
 
        return response;
    }
 
    /**
     * Update a Course.
     *
     * FIXME: what about uniqueness violations?
     *
     * @param id
     * @param req
     * @return
     */
    @Path("/{courseId}")
    @POST
    @Consumes({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_XML })
    public Response updateCourse(@PathParam("courseId") String id, CourseInfo req) {
 
        final String name = req.getName();
 
        // 'object not found' handled by AOP
        final Course course = finder.findCourseByUuid(id);
        final Course updatedCourse = manager.updateCourse(course, name, req.getSummary(), req.getDescription(),
                req.getCreditHours());
        final Response response = Response.ok(scrubCourse(updatedCourse)).build();
 
        return response;
    }
 
    /**
     * Delete a Course.
     *
     * @param id
     * @return
     */
    @Path("/{courseId}")
    @DELETE
    public Response deleteCourse(@PathParam("courseId") String id, @PathParam("version") Integer version) {
 
        // we don't use AOP handler since it's okay for there to be no match
        try {
            manager.deleteCourse(id, version);
        } catch (ObjectNotFoundException exception) {
            LOG.debug("course not found: " + id);
        }
 
        final Response response = Response.noContent().build();
 
        return response;
    }
}

Модульные тесты

Модульные тесты требуют внесения изменений в каждый тест, поскольку мы не можем просто создать экземпляры тестируемых объектов — мы должны использовать Spring, чтобы классы AOP были правильно сплетены. К счастью, это, по сути, единственное изменение — мы получаем Resource и устанавливаем сервисы с помощью метода package-private, а не через конструкторы package-private

Нам также необходимо создать значения Spring для наших служебных компонентов. Класс Configurer позаботится об этом.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
@Configuration
@ComponentScan(basePackages = { "com.invariantproperties.sandbox.student.webservice.server.rest" })
@ImportResource({ "classpath:applicationContext-rest.xml" })
// @PropertySource("classpath:application.properties")
public class TestRestApplicationContext1 {
 
    @Bean
    public CourseFinderService courseFinderService() {
        return null;
    }
 
    @Bean
    public CourseManagerService courseManagerService() {
        return null;
    }
 
    ....

Интеграционные тесты

Интеграционные тесты не требуют никаких изменений.

Исходный код

35
Share: