Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing GORM domain inheritance with Groovy 4: The interface XXX cannot be implemented more than once with different arguments 9.0.0-SNAPSHOT #1811

Closed
jamesfredley opened this issue Aug 22, 2024 · 4 comments · Fixed by #1834
Assignees

Comments

@jamesfredley
Copy link
Contributor

jamesfredley commented Aug 22, 2024

Domain inheritance can be solved in at least one of three ways.

  1. Change 1 exception to a warning in org.codehaus.groovy.classgen.Verifier.checkForDuplicateInterfaces(final ClassNode cn)
    The change on Groovy would be lines 367-368, going from exception to a warning output to console. After that change, the code with domain inheritance compiles, as it did in Groovy 3 and runs without issue. https://github.com/apache/groovy/blob/ab5a811f5bffecf439714bbc9b073c3f66acf42c/src/main/java/org/codehaus/groovy/classgen/Verifier.java#L350-L377
    this has also come up on several other Grails modules and the changes to work around it could be undone if this happens.
  2. Rewrite all of the Interfaces with Generic Type Parameters in GORM (grails data mapping) to AST instead, this is massive change, see below mock code to understand the scope better and attached decompiled domain classes in comment. This impacts not only compile time, but also runtime dynamic missing method interception for methods like findByNameAndAge("james", 56)
  3. Use Gradle Artifact Transforms to replace org.codehaus.groovy.classgen.Verifier in the Groovy distribution jar with the change from option 1, before it is used to compile domain classes. This can be done as a Gradle Plugin and could live in grails-gradle-plugin. See rough example in comment. This is only required at compile time. The compiled classes run without issue on standard Groovy 4.

Groovy-5106 (domain inheritance) issues: https://issues.apache.org/jira/browse/GROOVY-5106

The interface GormEntity cannot be implemented more than once with different arguments: org.grails.datastore.gorm.GormEntity<grails.gorm.tests.XXX> and org.grails.datastore.gorm.GormEntity<grails.gorm.tests.XXX>

This also applies to other projects with inheritance tests
https://github.com/search?q=org%3Agrails%20%2F*extends&type=code

https://github.com/search?q=org%3Agrails+%2F%2F%40Entity&type=code

I dug a bit more into a generated domain with a parent domain and each will have two generic traits with one trait implementing a generic interface and then the generic type parameter is used in many methods in GormEntity, which then forks out in a number of directions. Here is an very simplified view of what that looks like.
At least the first two traits and first interface will generate "The interface XXX cannot be implemented more than once with different arguments" error when the child extends the parent. The compiler stops at the first one it bumps into.

package com.example

import groovy.transform.CompileStatic

static void main(String[] args) {
    List<Child> c = Child.getAll() 
    List<Foo> f = Child.getAll()
    List<Parent> p = Parent.getAll()
}

@CompileStatic
class Parent implements GormEntity<Parent>, Entity<Parent> {
    Parent save(){
    }
}

@CompileStatic
class Child extends Parent implements GormEntity<Child>, Entity<Child> {

}

@CompileStatic
class Foo implements GormEntity<Foo>, Entity<Foo> {
    Foo save(){
    }
}

@CompileStatic
// org.grails.datastore.gorm.GormEntityApi
// API for instance methods defined by a GORM entity
interface GormEntityApi<D> {
    D save()
}

@CompileStatic
// grails.gorm.Entity
// Trait added to all domain classes
trait Entity<D> {

}

@CompileStatic
// org.grails.datastore.gorm.GormEntity
// A trait that turns any class into a GORM entity
trait GormEntity<D> implements GormEntityApi<D> {

    static List<D> getAll() {
        []
    }

    private GormInstanceApi<D> currentGormInstanceApi() {
        // more complex in reality, but same effect
        new GormInstanceApi<D>()
    }

    static GormQueryOperations<D> getNamedQuery(String queryName, Object...args) {
        new GormQueryOperations<D>()
    }

    private static GormStaticApi<D> currentGormStaticApi() {
        new GormStaticApi<D>()
    }
}

class GormInstanceApi<D> extends AbstractGormApi<D> implements GormInstanceOperations<D> {

}

abstract class AbstractGormApi<D> extends AbstractDatastoreApi {

}

abstract class AbstractDatastoreApi {

}

interface GormInstanceOperations<D> {

}

// interface in reality
class GormQueryOperations<T> {

}

class GormStaticApi<D> extends AbstractGormApi<D> implements GormAllOperations<D> {

}

interface GormAllOperations<D> extends GormStaticOperations<D>, GormInstanceOperations<D> {

}

interface GormStaticOperations<D> {

}
@jamesfredley jamesfredley changed the title New Failing Tests on 9.0.0 New Failing Tests on grails-data-mapping:9.0.0-snapshot Sep 12, 2024
@jamesfredley jamesfredley moved this to Todo in Grails 7 Sep 12, 2024
@jamesfredley jamesfredley changed the title New Failing Tests on grails-data-mapping:9.0.0-snapshot Failing Groovy-5106 (domain inheritance) Tests on grails-data-mapping:9.0.0-snapshot Sep 12, 2024
@jamesfredley jamesfredley changed the title Failing Groovy-5106 (domain inheritance) Tests on grails-data-mapping:9.0.0-snapshot Failing Domain inheritance Tests on grails-data-mapping:9.0.0-snapshot Sep 12, 2024
@jamesfredley
Copy link
Contributor Author

jamesfredley commented Oct 17, 2024

Example decompiled domain classes with inheritance:

Vehicle class.txt
Truck class.txt
Minivan class.txt

@jamesfredley
Copy link
Contributor Author

jamesfredley commented Oct 17, 2024

Gradle Artifact Transforms rough POC based on:

this rough POC currently puts the Verifier class files from Gradle's copy of Groovy (3.0.21) into the Groovy jar. It should use alternate versions compiled from the latest Groovy 4.0.x.

import org.gradle.api.artifacts.transform.InputArtifact
import org.gradle.api.artifacts.transform.TransformAction
import org.gradle.api.artifacts.transform.TransformOutputs
import org.gradle.api.artifacts.transform.TransformParameters
import org.gradle.api.file.ArchiveOperations
import org.gradle.api.file.FileSystemLocation
import org.gradle.api.provider.Provider
import java.util.jar.JarEntry
import java.util.jar.JarInputStream
import java.util.jar.JarOutputStream
import javax.inject.Inject
import static org.gradle.api.artifacts.type.ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE    

buildscript {
    repositories {
        maven { url "https://repo.grails.org/grails/core" }
        maven { url "https://plugins.gradle.org/m2/" }
        mavenLocal()
    }
    dependencies {
        classpath "javax.inject:javax.inject:1"
    }
}

configurations.named("compileClasspath") {
        attributes {
            attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, "transformed-jar")
        }
    }

    configurations.named("runtimeClasspath") {
        attributes {
            attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, "transformed-jar")
        }
    }

dependencies {
        registerTransform(JavaModuleTransform) {
            from.attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, "jar")
            to.attribute(ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE, "transformed-jar")
        }
    }


abstract class JavaModuleTransform implements TransformAction<TransformParameters.None> {

    @InputArtifact
    abstract Provider<FileSystemLocation> getInputArtifact()

    @Inject
    abstract ArchiveOperations getArchiveOperations()

    private List<String> replaceFiles = ['org/codehaus/groovy/classgen/Verifier.class',
                                 'org/codehaus/groovy/classgen/Verifier$1.class',
                                 'org/codehaus/groovy/classgen/Verifier$2.class',
                                 'org/codehaus/groovy/classgen/Verifier$3.class',
                                 'org/codehaus/groovy/classgen/Verifier$4.class',
                                 'org/codehaus/groovy/classgen/Verifier$5.class',
                                 'org/codehaus/groovy/classgen/Verifier$6.class',
                                 'org/codehaus/groovy/classgen/Verifier$7.class',
                                 'org/codehaus/groovy/classgen/Verifier$8.class',
                                 'org/codehaus/groovy/classgen/Verifier$9.class',
                                 'org/codehaus/groovy/classgen/Verifier$10.class',
                                 'org/codehaus/groovy/classgen/Verifier$11.class',
                                 'org/codehaus/groovy/classgen/Verifier$12.class',
                                 'org/codehaus/groovy/classgen/Verifier$13.class',
                                 'org/codehaus/groovy/classgen/Verifier$14.class',
                                 'org/codehaus/groovy/classgen/Verifier$15.class',
                                 'org/codehaus/groovy/classgen/Verifier$DefaultArgsAction.class',
                                 'org/codehaus/groovy/classgen/Verifier$SwapInitStatement$SwapInitInstruction.class',
                                 'org/codehaus/groovy/classgen/Verifier$SwapInitStatement.class',


    ]

    void writeEntry(JarOutputStream output, JarEntry jarEntry, JarInputStream input) {
        def jarEntryName = jarEntry.toString()
        if(jarEntryName in replaceFiles ) {
            output.putNextEntry(new JarEntry(jarEntryName))
            output.write(this.class.getResourceAsStream(
                    jarEntryName).readAllBytes())
        } else {
            output.putNextEntry(jarEntry)
            output.write(input.readAllBytes())
        }
        output.closeEntry()

    }

    @Override
    void transform(TransformOutputs outputs) {
        def jarName = inputArtifact.get().asFile.name

        if (!(jarName ==~ /^groovy-[0-9].[0-9].[0-9][0-9].jar$/)) {
            outputs.file(inputArtifact)
        } else {
            def originalJar = getInputArtifact().get().asFile
            def originalJarName = originalJar.name.substring(0, originalJar.name.lastIndexOf('.'))
            def target = outputs.file(jarName.replace(".jar", "-transformed.jar"))

            new JarInputStream(new FileInputStream(originalJar)).withStream { input ->
                new JarOutputStream(new FileOutputStream(target), input.manifest).withStream { output ->
                    var jarEntry = input.nextJarEntry

                    while (jarEntry != null) {
                        writeEntry(output, jarEntry, input)
                        output.closeEntry()
                        jarEntry = input.nextJarEntry
                    }

                    output.closeEntry()
                }
            }
        }
    }
}

@jamesfredley jamesfredley self-assigned this Oct 17, 2024
@jamesfredley jamesfredley changed the title Failing Domain inheritance Tests on grails-data-mapping:9.0.0-snapshot GORM domain inheritance with Groovy 4: The interface XXX cannot be implemented more than once with different arguments Oct 17, 2024
@jamesfredley jamesfredley changed the title GORM domain inheritance with Groovy 4: The interface XXX cannot be implemented more than once with different arguments Failing GORM domain inheritance with Groovy 4: The interface XXX cannot be implemented more than once with different arguments Oct 19, 2024
@jamesfredley jamesfredley changed the title Failing GORM domain inheritance with Groovy 4: The interface XXX cannot be implemented more than once with different arguments Failing GORM domain inheritance with Groovy 4: The interface XXX cannot be implemented more than once with different arguments 9.0.0-SNAPSHOT Oct 19, 2024
@jdaugherty
Copy link
Contributor

This is a blocker for Grails 7 release.

@jdaugherty
Copy link
Contributor

Opened upstream groovy ticket: https://issues.apache.org/jira/browse/GROOVY-11508

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants