Skip to content
This repository has been archived by the owner on Sep 14, 2022. It is now read-only.

Upgrade to Play Framework 2.8 #220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tarmath
Copy link

@tarmath tarmath commented Mar 1, 2020

Upgrade to Play Framework 2.8

Swagger Play Version | Play Version | Branch
---------------------| ------------ | ------
3.0.0 | 2.8 | [master](https://github.com/swagger-api/swagger-play/tree/master)
2.0.0 | 2.7 | [play27](https://github.com/swagger-api/swagger-play/tree/play-2.7)
Copy link
Author

Choose a reason for hiding this comment

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

assuming branch will be created after this PR is merged...

@tarmath
Copy link
Author

tarmath commented Mar 1, 2020

@webron @frantuma Can I get one of you to take a quick peek at this PR?

We've been using these changes internally for a couple weeks now without any issues.
Happy to adjust this as you see fit!

Much Appreciated!

@webron webron requested a review from frantuma March 2, 2020 16:52
@tarmath
Copy link
Author

tarmath commented Mar 22, 2020

resolves #213

@targeter
Copy link
Contributor

any chance this will be merged soon?
@tarmath did you happen to try cross-compilation to Scala 2.13?

@tarmath
Copy link
Author

tarmath commented Apr 24, 2020

@targeter cross compilation works great.

@gaeljw
Copy link

gaeljw commented Apr 29, 2020

No offence but what are we waiting to merge this?

@targeter
Copy link
Contributor

@webron @frantuma please take some action on this?

feguiguren added a commit to spokencloud/swagger-play that referenced this pull request May 29, 2020
ENG-14940: Add Play 2.8 support from swagger-api#220
@tarmath
Copy link
Author

tarmath commented Jun 6, 2020

@frantuma @webron could you guys please take a minute and let us know whether this project has been abandoned? perhaps that may lead to someone else rising up, forking and taking over maintenance / development of it?

much appreciated!

@webron
Copy link
Contributor

webron commented Jun 8, 2020

@tarmath it's not that it is abandoned as much as a capacity problem. I've tried reaching out to a few people in the community to become committers, but unfortunately have managed to get nobody yet. If anyone is interested, feel free to drop me a line (email in profile).

@arunzn
Copy link

arunzn commented Oct 17, 2020

Can you please merge it asap and release it for public use ?

@JayZYan
Copy link

JayZYan commented Oct 23, 2020

Anyone is going to merge this?

@gaeljw
Copy link

gaeljw commented Oct 23, 2020

Note that the plugin works fine with Play Framework 2.8 even though this PR is not merged. We have been running on the latest play-swagger release and Play Framework 2.8 for months now without issues.

@tolomaus
Copy link

Hi @gaeljw,

I did exactly what you suggested but I ran into an issue with fasterxml: it's throwing an error while generating the swagger.json because of a cyclic dependency on the StringProperty's readOnly method:

    public StringProperty readOnly() {
        this.setReadOnly(Boolean.TRUE);
        return this;
    }

I guess it's because it returns itself.

I found a similar unresolved issue on SO: https://stackoverflow.com/questions/62127929/swagger-ui-with-play-framework

I wonder why it worked just fine for you.

I'm currently on Play 2.8.2

Other verions:
io.swagger:swagger-annotations:1.5.22
io.swagger:swagger-core:1.5.22
io.swagger:swagger-models:1.5.22
io.swagger:swagger-play2_2.12:1.7.1
io.swagger:swagger-scala-module_2.12:1.0.5
com.fasterxml.jackson.core:jackson-annotations:2.11.0
com.fasterxml.jackson.core:jackson-core:2.11.0
com.fasterxml.jackson.core:jackson-databind:2.10.5
com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.10.3
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.9.8
com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.10.5
com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.10.5
com.fasterxml.jackson.module:jackson-module-parameter-names:2.10.3
com.fasterxml.jackson.module:jackson-module-paranamer:2.10.3
com.fasterxml.jackson.module:jackson-module-scala_2.12:2.10.3

@gaeljw
Copy link

gaeljw commented Oct 26, 2020

Hi @tolomaus , you should probably ensure that all Jackson dependencies are the same version (2.10.x, 2.11 doesn't work with Swagger AFAIK) with your build tool

@tolomaus
Copy link

@gaeljw I was finally able to work around the issue when I forced all fasterxml versions to 2.10.2!

Play 2.8.1 works fine out-of-the-box, but for Play 2.8.2 the fasterxml versions are bumped to 2.10.5 and that's when the issues surfaces.

I wonder why fasterxml 2.10.5 trips over the cyclic dependency, or rather, why 2.10.2 doesn't ;-)

@holthauj
Copy link

holthauj commented Dec 2, 2020

Here is a workaround that seems to resolve the cyclic dependency by ignoring the readOnly() method.

import io.swagger.models.properties.AbstractProperty;
import io.swagger.models.properties.Property;

static {
	ObjectMapper mapper = io.swagger.util.Json.mapper();
	mapper.addMixIn(AbstractProperty.class, AbstractPropertyMixin.class);
}

public static abstract class AbstractPropertyMixin {
	@JsonIgnore
	public abstract Property readOnly();
}

@felipebonezi
Copy link

@tarmath it's not that it is abandoned as much as a capacity problem. I've tried reaching out to a few people in the community to become committers, but unfortunately have managed to get nobody yet. If anyone is interested, feel free to drop me a line (email in profile).

Hi Webron, I want to help the community and become a committer.
I've sent you an email. Please, let me help you guys!

P.S.: Merge this commit 🙏🏻

@dwickern
Copy link
Contributor

dwickern commented Feb 4, 2021

I published my fork which cross-compiles for 2.7 and 2.8

// for Play 2.7:
libraryDependencies += "com.github.dwickern" %% "swagger-play2.7" % "3.0.0"

// for Play 2.8:
libraryDependencies += "com.github.dwickern" %% "swagger-play2.8" % "3.0.0"

@cleliofs
Copy link

cleliofs commented Mar 8, 2021

Hi All,

Any idea when this PR will get merged back to swagger-api:master?

@alejandrod-f
Copy link

Waiting for this merge to start using it! thanks

@AlexITC
Copy link

AlexITC commented Apr 2, 2022

@holthauj I ended up creating a java patch which solved a similar problem I had because a dependency is bringing jackson 2.11 to my classpath.

Tested with https://github.com/dwickern/swagger-play in play 2.8.7, thanks to @holthauj @dwickern

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.swagger.models.properties.AbstractProperty;
import io.swagger.models.properties.Property;

// See https://github.com/swagger-api/swagger-play/pull/220#issuecomment-736987055
public class JavaJacksonPatch {
    static {
        ObjectMapper mapper = io.swagger.util.Json.mapper();
        mapper.addMixIn(AbstractProperty.class, AbstractPropertyMixin.class);
    }

    public static void init() {
        System.out.println("Applying jackson patch");
    }

    public static abstract class AbstractPropertyMixin {
        @JsonIgnore
        public abstract Property readOnly();
    }
}

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.