这是Project Student的一部分。
许多人坚信方法应适合您的编辑器窗口(例如20行),而有些人则认为方法应小于此范围。 这个想法是一种方法应该做一件事,而只能做一件事。 如果它做的还不止于此,则应将其分解为多种方法,而旧方法的“一件事”就是协调新方法。
这并不意味着在任意数量的行之后拆分一种方法。 有时方法自然会更大。 仍然是一个很好的问题。
那么,如何识别不只一件事的代码? 一个好的试金石是代码是否在多种方法中重复。 典型的例子是持久性类中的事务管理。 每个持久性类都需要它,并且代码始终看起来相同。
另一个示例是Resource类中未处理的异常处理程序。 每个面向REST的方法都需要处理此问题,并且代码始终看起来相同。
那是理论。 在实践中,代码可能很丑陋并且收益不大。 幸运的是,有一个解决方案:面向方面的编程(AOP)。 这使我们可以在方法调用之前或之后透明地编织代码。 这通常使我们可以大大简化我们的方法。
设计决策
AspectJ –我正在通过Spring注入来使用AspectJ。
局限性
使用CRUD方法,AspectJ切入点表达式相对简单。 当添加了更复杂的功能时,情况可能并非如此。
资源方法中未处理的异常
我们首先关心的是资源方法中未处理的异常。 不管怎样,Jersey都会返回SERVER INTERNAL ERROR(服务器内部错误)(500)消息,但是它可能包含堆栈跟踪信息和我们不希望攻击者知道的其他内容。 如果我们自己发送它,我们可以控制它包含的内容。 我们可以在所有方法中添加一个“ catch”块,但可以将其复制到AOP方法中。 这将使我们所有的Resource方法更加苗条和易于阅读。
此类还检查“找不到对象”异常。 在单个Resource类中将很容易处理,但会使代码混乱。 将异常处理程序放在此处可使我们的方法专注于快乐路径并保证响应的一致性。
该类有两个优化。 首先,它显式检查UnitTestException并在这种情况下跳过详细的日志记录。 我最大的烦恼之一是测试,当一切都按预期方式运行时,将堆栈跟踪信息充斥日志。 这使得不可能针对明显的问题浏览日志。 单个更改可以使问题更容易发现。
其次,它使用与目标类(例如CourseResource)关联的记录器,而不是与AOP类关联的记录器。 除了更清晰之外,这还使我们可以有选择地更改单个Resource(而不是全部)的日志记录级别。
另一个技巧是在处理程序中调用ExceptionService 。 该服务可以对异常做一些有用的事情,例如,它可以创建或更新Jira票证。 这还没有实现,所以我只是发表评论以说明它的去向。
@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发布值检查
我们的Resource方法也有很多样板代码来检查REST参数。 它们是否为非空,电子邮件地址的格式是否正确,等等。同样,很容易将大部分代码移入AOP方法并简化Resource方法。
我们首先定义一个接口,该接口指示可以验证REST传输对象。 第一个版本使我们可以简单地接受或拒绝,改进的版本可以使我们有办法告诉客户具体问题是什么。
public interface Validatable {
boolean validate();
}
现在,我们扩展了先前的REST传输对象,以添加一种验证方法。
两个笔记。 首先,名称和电子邮件地址接受Unicode字母,而不仅仅是标准ASCII字母。 随着我们的世界国际化,这一点很重要。
其次,我添加了一个toString()方法,但是由于它使用了未经处理的值,因此这是不安全的。 我将在稍后处理消毒。
@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类。
@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搜索AOP类。 这是对我们的配置文件的单行更改。
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:aop="http://www.springframework.org/schema/aop"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
http://www.springframework.org/schema/context
http://www.springframework.org/schema/context/spring-context-3.0.xsd
http://www.springframework.org/schema/aop
http://www.springframework.org/schema/aop/spring-aop-3.0.xsd">
<aop:aspectj-autoproxy/>
</beans>
更新资源
现在,我们可以简化资源类。 仅有几种方法可以简化为幸福道路。
@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类。 幸运的是,这实际上是唯一的更改–我们检索资源并通过package-private方法而不是package-private构造函数设置服务。
我们还需要为服务bean创建Spring值。 配置器类负责此工作。
@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;
}
....
整合测试
集成测试不需要任何更改。
源代码
- 源代码位于https://github.com/beargiles/project-student [github]和http://beargiles.github.io/project-student/ [github页面]。
翻译自: https://www.javacodegeeks.com/2014/01/project-student-simplifying-code-with-aop.html