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

Change private properties to protected #12

Closed
rotexdegba opened this issue Sep 9, 2020 · 9 comments
Closed

Change private properties to protected #12

rotexdegba opened this issue Sep 9, 2020 · 9 comments
Assignees

Comments

@rotexdegba
Copy link

rotexdegba commented Sep 9, 2020

Hi There,

I am using this package in a web-application I am developing which is supposed to return system information in json format to various clients. This package does not work in windows when I run my application via the php builtin web-server. It however works when I run a script using this package on the command-line. My Powershell execution policy is good:

PS C:\Users\rotex> Get-ExecutionPolicy -List

Scope              ExecutionPolicy
----------------------------------
MachinePolicy  Undefined
UserPolicy        Undefined
Process            Undefined
CurrentUser     Unrestricted
LocalMachine  Undefined

When I ran my application via the php builtin web-server (via php -S 0.0.0.0:8880 -t .), I added the code below to Line 45 of \Ginfo\OS\Windows:

var_dump(($process->getErrorOutput()));

and it generated the output below:

'chcp' is not recognized as an internal or external command, operable program or batch file.

When I run a script (test.php, which is in the same folder as my web-application) below via php test.php on the command-line, it works correctly:

<?php
include_once './vendor/autoload.php';

$ginfo = new \Ginfo\Ginfo();
$info = $ginfo->getInfo();

var_dump($info->getBattery());

var_dump($info->getCpu());

(!is_null($info->getCpu())) && var_dump($info->getCpu()->getProcessors());

var_dump($info->getDisk());

(!is_null($info->getDisk())) && var_dump($info->getDisk()->getRaids());

(!is_null($info->getDisk())) && var_dump($info->getDisk()->getMounts());

(!is_null($info->getDisk())) && var_dump($info->getDisk()->getDrives());

var_dump($info->getGeneral());

var_dump($info->getMemory());

var_dump($info->getNetwork());

var_dump($info->getPci());

var_dump($info->getPhp());

var_dump($info->getPrinters());

var_dump($info->getProcesses());

var_dump($info->getSamba());

(!is_null($info->getSamba())) && var_dump($info->getSamba()->getConnections());

(!is_null($info->getSamba())) && var_dump($info->getSamba()->getFiles());

(!is_null($info->getSamba())) && var_dump($info->getSamba()->getServices());

var_dump($info->getSelinux());

var_dump($info->getSensors());

var_dump($info->getServices());

var_dump($info->getSoundCard());

var_dump($info->getUps());

var_dump($info->getUsb());

My Operating System is Windows 10 Home Version 2004 (OS Build 19041.508). My PHP info:

PHP 7.2.31 (cli) (built: May 12 2020 10:26:32) ( ZTS MSVC15 (Visual C++ 2017) x64 )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Xdebug v2.9.6, Copyright (c) 2002-2020, by Derick Rethans

To get my web-application to work properly with this package on Windows 10, I had to write a new sub-class of \Ginfo\OS\Windows. The code for this sub-class follows below:

<?php

class ExtendedWindows extends \Ginfo\OS\Windows {
    
    public function __construct()
    {
        ini_set('max_execution_time', 0);
        parent::__construct();
    }

    protected function getInfo(string $name): ?array
    {
//        if (\array_key_exists($name, $this->infoCache)) {
//            
//            return $this->infoCache[$name];
//        }

        $result = \json_decode(
            shell_exec('chcp 65001 | powershell -file '. S3MVC_APP_ROOT_PATH.'\\vendor\\gemorroj\\ginfo\\bin\\windows\\'.$name.'.ps1'), 
            true
        );

        return \is_scalar($result) ? [$result] : $result;
    }
}

This sub-class works for me and it uses the native php function shell_exec instead of SymfonyProcess used in \Ginfo\OS\Windows. I guess the issue is with SymfonyProcess. You made the $infoCache and $powershellDirectory properties of \Ginfo\OS\Windows private, and as a result, I can't use them in my sub-class.
Could you please change them from private to protected or do you want me to create a pull request with these changes?

private $infoCache = [];

private $powershellDirectory;

Thanks for your anticipated cooperation,

Rotimi

NOTE: The package works fine in my CentOS 7 Linux environment, by the way.

@rotexdegba rotexdegba changed the title Change of private properties to protected Change private properties to protected Sep 9, 2020
@Gemorroj
Copy link
Owner

@rotexdegba I can add setPowershellDirectory(string $path): self and cleanInfoCache(): void methods. Will it helps you?

@rotexdegba
Copy link
Author

rotexdegba commented Sep 10, 2020

Below are the methods I would need you to add to \Ginfo\OS\Windows.

protected function setPowershellDirectory(string $path): self 
{
    $this->powershellDirectory = $path;

    return $this;
}

protected function addToInfoCache(string $name, $value): self 
{
    $this->infoCache[$name] = $value;

    return $this;
}

protected function infoCacheContains(string $name): bool 
{
   return \array_key_exists($name, $this->infoCache);
}

protected function getFromInfoCache(string $name, $defaultValue=null) 
{
    return $this->infoCacheContains($name) ? $this->infoCache[$name]: $defaultValue;
}

protected function removeFromInfoCache(string $name):self 
{
    if ($this->infoCacheContains($name)) {
        unset($this->infoCache[$name]);
    }
  
    return $this;
}

protected function cleanInfoCache(): self
{
    $this->infoCache = [];
    
    return $this;
}

Do you think they should be public or protected?

Thanks

@Gemorroj
Copy link
Owner

@rotexdegba idk, at this moment i think it's not important. we can start with protected visibility

@rotexdegba
Copy link
Author

rotexdegba commented Sep 10, 2020

@Gemorroj OK that sounds good.

@rotexdegba
Copy link
Author

@Gemorroj do you want me to send you a pull request or are you going to do it yourself?

@Gemorroj
Copy link
Owner

@rotexdegba can you try the master branch?

@Gemorroj Gemorroj self-assigned this Sep 10, 2020
@rotexdegba
Copy link
Author

rotexdegba commented Sep 10, 2020

@Gemorroj Thanks, I have pulled the latest changes from master and it works as expected. Do you know when you will be tagging another release with all your most recent changes (I am uncomfortable specifying dev-master in my composer.json and would like to switch from "gemorroj/ginfo": "dev-master" back to "gemorroj/ginfo": "^5.1" in my composer.json)?

@Gemorroj
Copy link
Owner

Gemorroj commented Sep 11, 2020

@rotexdegba i'd released 5.2.0 version

@rotexdegba
Copy link
Author

rotexdegba commented Sep 11, 2020

Thanks a million for your quick response. Everything works fine now.

Cheers

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

No branches or pull requests

2 participants