Skip to content

BAEL-1085#2428

Merged
zhendrikse merged 12 commits intoeugenp:masterfrom
Thoughtscript:master
Sep 3, 2017
Merged

BAEL-1085#2428
zhendrikse merged 12 commits intoeugenp:masterfrom
Thoughtscript:master

Conversation

@Thoughtscript
Copy link
Copy Markdown
Contributor

No description provided.

@@ -0,0 +1,20 @@
package net.baeldung;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

com.baeldung :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Normally, the convention is com, org, then net (used rarely). I thought it easier to simply and cleanly add my code into a separate grouping. However, I think I'm supposed to move this over to Spring Angular instead. So... my point is moot.

import org.springframework.boot.web.support.SpringBootServletInitializer;

@SpringBootApplication
public class ApplicationMain extends SpringBootServletInitializer {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need SpringBootServletInitializer? I believe that should work only with the @SpringBootApplication

@RequestMapping("/post")
public class ExamplePostController {

Logger log = LoggerFactory.getLogger(ExamplePostController.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

private static final log

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will make that change.

public class ExampleService {

public boolean fakeAuthenticate(LoginForm lf) {
return (lf.getUsername().equals(lf.getUsername()) && lf.getPassword() == lf.getPassword());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be easier to simply return true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah - it could be simplified - just wanted to flesh that out - here it does qualify as a kind of authentication check since it compares values.

private String username;
private String password;

public LoginForm() {};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's redundant

.permitAll()
.antMatchers("/secured/**/**",
"/secured/success", "/secured/socket", "/secured/success")
"/secured/success", "/secured/socket")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is that related?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No but this is a minor change requested elsewhere.

}
}

@After
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In @before you are assigning something to mockMvc - so this @after does not make any impact and can be safely removed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True but leaving the full before, test, and after trio allows someone learning to see how a good test should be set up.

}
}

@After
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, you can remove that

@Thoughtscript
Copy link
Copy Markdown
Contributor Author

This code is being moved over to Spring-Rest-Angular per document review! Most of the points above are moot due to that change but some will be incorporated for sure. Thanks!

@Thoughtscript
Copy link
Copy Markdown
Contributor Author

Had removed a duplicate ant matcher from BAEL-509 - this was the incorrect process - reverting now, article has been corrected, but will issue a separate PR for this

@zhendrikse zhendrikse merged commit 4baaf9f into eugenp:master Sep 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants