Skip to content

Conversation

@ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Nov 3, 2023

Summary

Allows working with classes that might or might not be available. For safety I chose to only allow this if a constructor argument is typed.

I need this for a cross-app dependency inside of server in #40559.

Checklist

Allows working with classes that might or might not be available.

Signed-off-by: Christoph Wurst <[email protected]>
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Nice 👍

@devnoname120
Copy link

@ChristophWurst Is it supposed to work for null injections?

i.e.

  • I have a class PageController that requests an optional ?PreferenceService $preferenceService injection
  • PreferenceService requests string $userId (not optional)
  • The user is not connected so $userId cannot be injected (it's null)

What I would expect is that PreferenceService can't be built because $userId cannot be null, however PageController would be instantiated without issues because although PreferenceService couldn't be built, it's an optional dependency.

Here is my attempt: devnoname120/epubviewer@89a76c2

What I see is this instead:

{
  "reqId": "SA3HE70WauL60defOmaI",
  "level": 3,
  "time": "2025-02-26T01:17:09+00:00",
  "remoteAddr": "192.168.215.1",
  "user": "--",
  "app": "index",
  "method": "GET",
  "url": "/index.php/apps/epubviewer/?file=%2Findex.php%2Fs%2FrsQgKNjyNGqgYEP%2Fdownload&type=application%2Fepub%2Bzip",
  "message": "OCA\\Epubviewer\\Db\\BookmarkMapper::__construct(): Argument #2 ($userId) must be of type string, null given",
  "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Safari/537.36",
  "version": "30.0.6.2",
  "exception": {
    "Exception": "TypeError",
    "Message": "OCA\\Epubviewer\\Db\\BookmarkMapper::__construct(): Argument #2 ($userId) must be of type string, null given",
    "Code": 0,
    "Trace": [
      {
        "function": "__construct",
        "class": "OCA\\Epubviewer\\Db\\BookmarkMapper",
        "type": "->",
        "args": [
          {
            "__class__": "OC\\DB\\ConnectionAdapter"
          },
          null,
          {
            "__class__": "OCA\\Epubviewer\\Utility\\Time"
          }
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 98,
        "function": "newInstanceArgs",
        "class": "ReflectionClass",
        "type": "->",
        "args": [
          [
            {
              "__class__": "OC\\DB\\ConnectionAdapter"
            },
            null,
            {
              "__class__": "OCA\\Epubviewer\\Utility\\Time"
            }
          ]
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 106,
        "function": "buildClass",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          {
            "__class__": "ReflectionClass",
            "name": "OCA\\Epubviewer\\Db\\BookmarkMapper"
          }
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 124,
        "function": "resolve",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Db\\BookmarkMapper"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/DependencyInjection/DIContainer.php",
        "line": 451,
        "function": "query",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Db\\BookmarkMapper"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/DependencyInjection/DIContainer.php",
        "line": 423,
        "function": "queryNoFallback",
        "class": "OC\\AppFramework\\DependencyInjection\\DIContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Db\\BookmarkMapper"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 74,
        "function": "query",
        "class": "OC\\AppFramework\\DependencyInjection\\DIContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Db\\BookmarkMapper",
          true
        ]
      },
      {
        "function": "OC\\AppFramework\\Utility\\{closure}",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 98,
        "function": "array_map",
        "args": [
          {
            "__class__": "Closure"
          },
          [
            "*** sensitive parameters replaced ***"
          ]
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 106,
        "function": "buildClass",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          {
            "__class__": "ReflectionClass",
            "name": "OCA\\Epubviewer\\Service\\BookmarkService"
          }
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 124,
        "function": "resolve",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Service\\BookmarkService"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/DependencyInjection/DIContainer.php",
        "line": 451,
        "function": "query",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Service\\BookmarkService"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/DependencyInjection/DIContainer.php",
        "line": 423,
        "function": "queryNoFallback",
        "class": "OC\\AppFramework\\DependencyInjection\\DIContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Service\\BookmarkService"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 74,
        "function": "query",
        "class": "OC\\AppFramework\\DependencyInjection\\DIContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Service\\BookmarkService",
          true
        ]
      },
      {
        "function": "OC\\AppFramework\\Utility\\{closure}",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 98,
        "function": "array_map",
        "args": [
          {
            "__class__": "Closure"
          },
          [
            {
              "__class__": "ReflectionParameter",
              "name": "appName"
            },
            {
              "__class__": "ReflectionParameter",
              "name": "request"
            },
            {
              "__class__": "ReflectionParameter",
              "name": "urlGenerator"
            },
            {
              "__class__": "ReflectionParameter",
              "name": "rootFolder"
            },
            {
              "__class__": "ReflectionParameter",
              "name": "shareManager"
            },
            "And 3 more entries, set log level to debug to see all entries"
          ]
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 106,
        "function": "buildClass",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          {
            "__class__": "ReflectionClass",
            "name": "OCA\\Epubviewer\\Controller\\PageController"
          }
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/Utility/SimpleContainer.php",
        "line": 124,
        "function": "resolve",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Controller\\PageController"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/DependencyInjection/DIContainer.php",
        "line": 451,
        "function": "query",
        "class": "OC\\AppFramework\\Utility\\SimpleContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Controller\\PageController"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/DependencyInjection/DIContainer.php",
        "line": 423,
        "function": "queryNoFallback",
        "class": "OC\\AppFramework\\DependencyInjection\\DIContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Controller\\PageController"
        ]
      },
      {
        "file": "/var/www/html/lib/private/AppFramework/App.php",
        "line": 140,
        "function": "query",
        "class": "OC\\AppFramework\\DependencyInjection\\DIContainer",
        "type": "->",
        "args": [
          "OCA\\Epubviewer\\Controller\\PageController"
        ]
      },
      {
        "file": "/var/www/html/lib/private/Route/Router.php",
        "line": 303,
        "function": "main",
        "class": "OC\\AppFramework\\App",
        "type": "::",
        "args": [
          "OCA\\Epubviewer\\Controller\\PageController",
          "showReader",
          {
            "__class__": "OC\\AppFramework\\DependencyInjection\\DIContainer"
          },
          {
            "_route": "epubviewer.page.showreader"
          }
        ]
      },
      {
        "file": "/var/www/html/lib/base.php",
        "line": 1003,
        "function": "match",
        "class": "OC\\Route\\Router",
        "type": "->",
        "args": [
          "/apps/epubviewer/"
        ]
      },
      {
        "file": "/var/www/html/index.php",
        "line": 24,
        "function": "handleRequest",
        "class": "OC",
        "type": "::",
        "args": []
      }
    ],
    "File": "/var/www/html/apps-extra/epubviewer/lib/Db/BookmarkMapper.php",
    "Line": 23,
    "message": "OCA\\Epubviewer\\Db\\BookmarkMapper::__construct(): Argument #2 ($userId) must be of type string, null given",
    "exception": {},
    "CustomMessage": "OCA\\Epubviewer\\Db\\BookmarkMapper::__construct(): Argument #2 ($userId) must be of type string, null given"
  }
}

devnoname120 added a commit to devnoname120/epubviewer that referenced this pull request Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement

Projects

Development

Successfully merging this pull request may close these issues.

6 participants