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

slove code issue #180

Closed
wants to merge 1 commit into from
Closed

slove code issue #180

wants to merge 1 commit into from

Conversation

visonforcoding
Copy link

@visonforcoding visonforcoding commented Nov 12, 2021

This request solves 2 problems
First, prevent unlimited generation of temporary class files. Ensure that one source code file corresponds to one generated file.
Secondly, 1 bug was solved.
When the method of the source code is bound, there is a Declaration error when the parameter is added or modified.
@RequestMapping is bound with aop.

    /**
     * @RequestMapping(name="test",path="/test",method="GET")
     */
    public function test()
    {
        return 'hello,hll1';
    }

    /**
     * @RequestMapping(name="add",path="/add",method="GET")
     * @LoginRequired
     */
    public function add4343(GoodsQuery $goodsQuery)
    {
        return 'hello,add';
    }

The corresponding file generated is Mall_Api_Controller_IndexCtrller_2956060156.php

Change the source code to

   /**
      * @RequestMapping(name="test",path="/test",method="GET")
      */
     public function test()
     {
         return'hello,hll1';
     }

     /**
      * @RequestMapping(name="test",path="/add",method="GET")
      * @LoginRequired
      */
     public function add4343()
     {
         return'hello,add';
     }

No new files will be generated.

We will get error

Declaration of Mall\Api\Controller\IndexCtrller_2956060156::add4343(Mall\Api\Controller\RequestMapper\GoodsQuery $goodsQuery) should be compatible with Mall\Api\Controller\IndexCtrller::add4343()

@sonarcloud
Copy link

sonarcloud bot commented Nov 12, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@koriym
Copy link
Member

koriym commented Nov 12, 2021

@koriym
Copy link
Member

koriym commented Dec 6, 2021

-       $aopClassName = ($this->aopClassName)($class, $bind->toString(''));
-       $classReflection = new ReflectionClass($class);
+       $aopClassName = sprintf('%s_%s', $class, filemtime($classReflection->getFileName()));

It detects changes in the source code, but not changes in the bindings.

@koriym
Copy link
Member

koriym commented Dec 6, 2021

I create the issue #182

@koriym
Copy link
Member

koriym commented Sep 9, 2022

I checked again.

If class generation for different bindings is done at the same runtime, this PR will cause problems. Sure it is a rare case, but compatibility is destroyed. Adding a file stamp to the current binding solves this problem.

Once generated, the class is checked for existence and will not be generated again.

@koriym
Copy link
Member

koriym commented Sep 10, 2022

@visonforcoding Will fix with #192

@koriym koriym closed this in #192 Sep 27, 2022
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.

None yet

2 participants