Skip to content
This repository has been archived by the owner on Feb 10, 2021. It is now read-only.

Migrate to play 2.6 #345

Closed
wants to merge 30 commits into from
Closed

Migrate to play 2.6 #345

wants to merge 30 commits into from

Conversation

KadekM
Copy link
Contributor

@KadekM KadekM commented Jul 9, 2017

For #343

joscha/play-easymail#53 is required, so this builds:

"com.feth" %% "play-easymail" % "0.9.3"

@@ -1,7 +1,8 @@
language: scala
sudo: true
scala:
- 2.11.8
- 2.11.11
Copy link
Owner

@joscha joscha Jul 11, 2017

Choose a reason for hiding this comment

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

I think play-easymail is only released for 2.12 :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, but it should be possible to release for 2.12... I don't know travis well, but
https://github.com/joscha/play-easymail/blob/master/.travis.yml

it should run the "publish" for both versions, or ?

Copy link
Owner

Choose a reason for hiding this comment

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

I cross-compiled 0.9.3, so we need to use that. I just hit publish in sonatype, will need a while until its available.

Copy link
Owner

Choose a reason for hiding this comment

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

change is in f1fc148

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool

@joscha
Copy link
Owner

joscha commented Jul 11, 2017

play-easymail 0.9.3 is not in sonatype, yet, will show up here: https://search.maven.org/#search%7Cga%7C1%7Cplay-easymail once published.

@joscha
Copy link
Owner

joscha commented Jul 11, 2017

added a couple more needed changes

@joscha
Copy link
Owner

joscha commented Jul 12, 2017

@KadekM There are a few problems with the latest version, can you have a look, please? See https://travis-ci.org/joscha/play-authenticate/jobs/252710668

Locally, you can make it work easily by cd code && sbt publish-local && cd ../samples/... && sbt run

@KadekM
Copy link
Contributor Author

KadekM commented Jul 12, 2017

@joscha yea I'll check as soon as I have a bit of time

@KadekM
Copy link
Contributor Author

KadekM commented Jul 12, 2017

we need to bump play ebean as well, so its a bit more work :|

@KadekM
Copy link
Contributor Author

KadekM commented Jul 12, 2017

@joscha there are some issues with code like circular deps which are now disallowed by play by default and I'm not sure if forcing it enabled onto clients is right way to go... just run example to see guice error

@KadekM
Copy link
Contributor Author

KadekM commented Jul 18, 2017

I can finish this sometimes in onwards from mid-august since until then I'm on vacation. We can start with non optimal solution of just enabling circular references (as it's now).

@joscha joscha mentioned this pull request Aug 21, 2017
@joscha
Copy link
Owner

joscha commented Aug 21, 2017

@KadekM any chance to wrap the work here up?

@KadekM
Copy link
Contributor Author

KadekM commented Aug 21, 2017

@joscha I'll look on it during weekend

@joscha
Copy link
Owner

joscha commented Aug 21, 2017 via email

@mkurz
Copy link
Collaborator

mkurz commented Mar 8, 2018

@mkurz
Copy link
Collaborator

mkurz commented Mar 12, 2018

Maybe the problem is not related to deadbolt?

@joscha
Copy link
Owner

joscha commented Mar 12, 2018 via email

@rozkerim
Copy link

rozkerim commented May 4, 2018

This doc says Ebean 3.x not suitable for Play 2.6, but at here still has depedency for Ebean 3.0.0.

@GBeushausen
Copy link

Play 2.7 M1 was just released. A 2.6 version is not available in over a year now. So we might consider this project dead, right?

@joscha
Copy link
Owner

joscha commented Jun 23, 2018

@GBeushausen if @rozkerim is right, the last missing piece for 2.6 support is Ebean. I have spent quite a bit of time to try and make it work, but couldn't find the ultimate reason 2.6 doesn't work as it should. As any opensource project, it relies a bit on contributions, so if you (or anyone else) find time to make it work with 2.6 (or 2.7 even) I am more than just a little happy to release a new version.

@Mule52
Copy link

Mule52 commented Jun 27, 2018

Is Ebean the problem at this point? Ebean needs to be replaced with JPA or something in order allow upgrading to 2.6 or 2.7? Or is it something else?

@leccyril
Copy link

no solution ? we are blocked to version 2.5 ? no one could help to make it work this great module ? the only one for playframework ?

alunev added a commit to alunev/freemoney that referenced this pull request Aug 6, 2018
@bravegag
Copy link
Contributor

Hi guys! I'm happy I was referred to this discussion. Please see my comments in #360 I give multiple alternatives moving forward and to get a 2.6 ready ASAP.

@Mule52
Copy link

Mule52 commented Aug 18, 2018

@bravegag Thanks for your comments. Your idea about removing Ebean in the samples was enough for me to do that locally this week in my app. I should be able to upgrade Play locally for my app. It was no easy task, but now I am free of Ebean. I am using Jooq in my app. I would think the samples could be rewritten to use any of those JPA examples you shared.

I am sharing this because I feel like rather than rewriting my app to use some other framework, I just stripped out Ebean and replaced it with Jooq, which I was using throughout my app anyway. I am using Deadbolt as well, and really the only gotcha was to make sure I have some class that implements Subject, Role, and Permission. The rest of my app can use my Jooq pojo for the user, while Deadbolt will use my "AppUser" class.

It seems like what is holding up this library from upgrading is Ebean in the samples. Thanks for your comments.

@GBeushausen
Copy link

@Mule52: I don't think this is a viable solution. Many people are using ebean. I have plenty of apps written for my customers using ebean. Having to rewrite all of these with another JPA solution is plain impossible. From my experience it's much easier to migrate to pac4j which i've done for my current projects.

@bravegag
Copy link
Contributor

bravegag commented Aug 19, 2018

@GBeushausen: starting from scratch with JPA can be daunting but I thought about this long time ago when I wrote perfectjpattern. Once I manage to get the "Remember me" PR in I will re-fork and rewrite Ebean to perfectjpattern's Hibernate or OpenJPA though the former makes life a lot easier e.g. with supporting the nice DAO findByExample. Here you don't need a DAO-per-model-type the generic DAO covers 90% of the use-cases.

I hope doing so will encourage you to switch to JPA and stay on PA.

Besides the "Remember Me" which I sponsored via Upwork, I'm thinking to also introduce Two-Factor authentication using Google's Authenticator App ... exciting stuff! and functionality that is not easy to develop from scratch for each application that builds on PA.

I feel your concerns but I still think that it is a lot easier to replace the DAO layer than coming up with a different Auth framework with all the features that PA has, also considering the user and contribution base it has.

@GBeushausen
Copy link

@bravegag: That's just not how software development works. My clients have spent 30K -60K for the app i developed for them. I cannot tell them to spend another 20K just for refactoring the ORM, so that the auth framework works with the latest framework version. They get no benefits, maybe like more bugs in the first place. So they would never release the budget for such a thing.

@bravegag
Copy link
Contributor

bravegag commented Aug 19, 2018

@GBeushausen following your reasoning then why migrate to Play 2.6? they also get no extra benefits functionality-wise, an old saying in SE: don't change what works. However, moving forward you could use the new ORM tech with new clients and new project. Finally, if it bothers you so much then do something about it, pay or develop whatever it takes to fix EBean.

Again to put things into perspective, PA doesn't need EBean. EBean is just a technology choice for the samples. I have never used EBean in any project before and don't need it.

@GBeushausen
Copy link

GBeushausen commented Aug 19, 2018

@bravegag: Migrating to Play 2.6 can have reasons customers are wiling to pay for. For example more req/s due to the new Akka HTTP server etc. There are a few things which i don't understand from your perspective. Why on earth are you planning to use such an exotic thing like "perfectjpattern"? There are only 4 questions on stack overflow about this software. Google trends doesn't even have enough data about it. This must be a joke right? I mean if you're serious about rewriting the ORM layer, why not at least use JPA / Hibernate directly? Or Spring data? But trying to get this project to use such an exotic freak that no one has ever heard of is certainly not a direction we want this project to go, do we?

@bravegag
Copy link
Contributor

bravegag commented Aug 20, 2018

@GBeushausen This is the last message I will write to you. If you are so successful, flashing and showing off earnings, and you are such a majesty in software development that you name-calling and prejudice against people and projects you don't know while the only thing I see you doing is whining and crying for features you're either unable to develop or afford. Things just don't add up.

I can sponsor or develop whatever feature I want, I really don't need you nor EBean, in my view, perfectjpattern is a good investment for my future projects but I'm also very fluent in SQL and Scala so also very happy using Slick. I will fork PA and make the samples work with perfectjpattern which is simply a thin layer on top of JPA that makes working on top of JPA and switching between JPA providers easy. Some folks will like it and use it, some folks like yourself will hate it, and that's fine.

@joscha
Copy link
Owner

joscha commented Aug 20, 2018

guys, please stay civil. I appreciate all your contributions but personal attacks can not be tolerated.

FWIW I am not sold 100% on Ebean, I have heard good things about jooq, but not a lot of experience with it, same with a few others.
Backwards compatibility and also some amount of the standard way of doing persistence with Play is important though, so I am a bit wary of throwing Ebean out and replacing it with something as unknown as perfectjpattern - the software might be great, I wouldn't know, I've never used it, but it would certainly be quite hard for anyone to use it unless we assumed they would adopt a new (quite unknown and seemingly not very widespread used ORM) together with an authentication library. I don't think that is a sellable package?

@schaloner
Copy link

schaloner commented Aug 20, 2018 via email

@GBeushausen
Copy link

@bravegag: I didn't want to sound rude, neither did i wanted to boast with my income, i just wanted to express the view of commercial software developers. So please don't take it personal.

I already offered to contribute financially to find a solution for this problem. Maybe someone can set up a crowd funding page or something.

@schaloner: Thanks for this information. This would be a heavy burden for this project, if commercial developers have to pay License fees for software they didn't even want to use. I think if this project really wanted to move away from Ebean, it should be either JPA or Spring data. If at all. Ebean is so common in the Play community.

@joscha
Copy link
Owner

joscha commented Aug 20, 2018

I just fired up https://github.com/joscha/play-authenticate/tree/play2.6-migration again to give EBean another spin - I am not even sure where it fails - tbh I am not even sure if it is EBean - the only thing I can say for certain is, that there is something wrong which makes deadbolt think a user is not authenticated. Please use the referenced branch if you want to help, any hints are appreciated!

@joscha
Copy link
Owner

joscha commented Aug 20, 2018

as a matter of fact with a tweaked logback I can now see a:

[warn] p.filters.CSRF - [CSRF] Check failed because no or invalid token found in body for /signup
[warn] p.filters.CSRF - [CSRF] Check failed with NoTokenInBody for /signup

best clue so far, will push my current state of the branch

@GBeushausen
Copy link

@joscha: Great, i'll give it a try, though i don't have much time. Btw, what happens if you disable security filters?

@joscha
Copy link
Owner

joscha commented Aug 20, 2018

I think I have it working via @helper.CSRF.formField - will push the changes, but it will need some cleanup I think.

@joscha joscha mentioned this pull request Aug 20, 2018
@joscha
Copy link
Owner

joscha commented Aug 20, 2018

I think #364 is working with 2.6 - will wait for CI - an awful lot of deprecation warnings though, mainly around the form helpers. Probably time to get rid of them soon. Would be good if someone else could give what is in #364 a spin and read over the changes.

@joscha
Copy link
Owner

joscha commented Aug 23, 2018

okay, #364 is almost ready to be merged - I can't get the Selenium tests to pass right now and they won't start locally for me for some reason (gecko driver?). Can anyone else give them a try?
You should be able to start and pass them via:

export GOOGLE_USER_PASSWORD="..." \
       GOOGLE_CLIENT_ID="..." \
       GOOGLE_CLIENT_SECRET="..." \
       EVENTBRITE_CLIENT_ID="..." \
       EVENTBRITE_CLIENT_SECRET="..." \
       EVENTBRITE_USER_PASSWORD="..." \
       FACEBOOK_USER_PASSWORD="..." \
       FACEBOOK_CLIENT_ID="..." \
       FACEBOOK_CLIENT_SECRET="..."
sbt --info test

@joscha joscha closed this in #364 Aug 27, 2018
@joscha
Copy link
Owner

joscha commented Aug 27, 2018

Version 0.9.0 for Play 2.6 should show up here soon: https://mvnrepository.com/artifact/com.feth/play-authenticate

I had to disable integration tests due to browser issues to get the release done. Let me know how you fare. Sorry it took so long, anyone reading here should probably put their hand up for becoming a maintainer, so we can avoid these delays in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.