PHP 7.0 - 'INIController' - A set of classes for reading, parsing, and editing .ini files directly or in memory (Second version)

Question

This is the second review of this particular project, after I've made changes based on feedback I received on my last post on code review, which you can take a look at here. Yes, I copy pasted some stuff from the previous post because it's still relevant to this version of the project.

I want to preface this by saying I am fully aware PHP already has functions for handling .ini files, and there's already libraries out there that do what this project does. The purpose here wasn't to make something new, it was to be a personal learning experience in OOP (This is my first time making an object oriented project in PHP), documentation (Via PHPDoc), unit tests with PHPUnit, and building a composer project.

The project's been condensed down into a single class, and a custom Exception handler that exists primarily to provide specific static error codes alongside a full human-readable error description.

The source code if you'd rather view it on GitHub.

The main INILib class:

This is the class that does all the actual work. I've condensed the original 3 classes into a single class that completely drops the idea of using statics. I've also replaced file operations with the SplFileObject library, and set up the __construct() method to require an input of an existing SplFileObject

<?php
/**
 * INI file parsing and manipulation library.
 *
 * @author SierraKomodo
 * @license GPL3
 */

namespace SierraKomodo\INILib;

/**
 * Primary INI library class
 *
 * Leveraging a provided SplFileObject pointing to an INI file, this class provides in-memory reading and modifying of
 *   data stored in INI files. Methods are also provided to write any changes made in memory to the INI file.
 *
 * @package SierraKomodo\INIController
 * @version 0.1.0-review.2 Peer review version 2. Currently in development; Not fully tested yet.
 */
class INILib
{
    /**
     * @var \SplFileObject The INI file being read and modified by this class
     * @used-by INILib::__construct()
     * @used-by INILib::parseINIData()
     */
    protected $fileObject;
    /**
     * @var array The contents of the INI file, converted to a multi-layer array (Same format as \parse_ini_file())
     * @used-by INILib::parseINIData()
     * @used-by INILib::generateFileContent()
     */
    protected $iniDataArray = array();

    const FETCH_STRING = 1;
    const FETCH_MIXED  = 2;


    /**
     * INILib constructor.
     *
     * @param \SplFileObject $parFile The INI file to initialize the object with
     * @param int $parScannerMode See parseINIData() parameter $parScannerMode
     * @uses INILib::$fileObject
     * @uses INILib::parseINIData()
     */
    public function __construct(\SplFileObject $parFile, int $parScannerMode = INI_SCANNER_NORMAL)
    {
        $this->fileObject = $parFile;
        $this->parseINIData($parScannerMode);
    }


    /**
     * Getter for $iniDataArray, to prevent arbitrary modifications to the array.
     *
     * @return array $this->$iniDataArray
     * @uses INILib::$iniDataArray
     */
    public function dataArray(): array
    {
        return $this->iniDataArray;
    }


    /**
     * Fetches a specified key=value pair from the data array. Alternative to using dataArray() to fetch the entire array
     *
     * @param string $parSection
     * @param string $parKey
     * @return mixed|null The requested value (Type dependent on scanner mode used in the last parseINIData() call), or
     *   NULL if no matching entry was found
     */
    public function fetchEntry(string $parSection, string $parKey)
    {
        // If the entry is empty, return null
        if (empty($this->iniDataArray[$parSection][$parKey])) {
            return null;
        }

        // Return the value
        return $this->iniDataArray[$parSection][$parKey];
    }


    /**
     * Reads the INI file and stores the contents into memory as a multi-layered array.
     *
     * Any 'unsaved changes' to the INI data in memory are lost.
     *
     * @param int $parScannerMode One of INI_SCANNER_NORMAL, INI_SCANNER_RAW, INI_SCANNER_TYPED.
     *   Defaults to INI_SCANNER_NORMAL. See Parameters > scanner_mode here:
     *   http://php.net/manual/en/function.parse-ini-string.php
     * @uses INILib::$fileObject
     * @uses INILib::$iniDataArray
     * @throws INILibException if the file could not be locked, read, or parsed
     */
    public function parseINIData(int $parScannerMode = INI_SCANNER_NORMAL)
    {
        // Lock the file for reading
        if ($this->fileObject->flock(LOCK_SH) === false) {
            throw new INILibException(
                "Failed to acquire a shared file lock",
                INILibException::ERR_FILE_LOCK_FAILED
            );
        }

        // Set pointer to start of file
        $this->fileObject->rewind();

        // Pull the file's contents
        $fileContents = $this->fileObject->fread($this->fileObject->getSize());
        if ($fileContents === false) {
            $this->fileObject->flock(LOCK_UN);
            throw new INILibException(
                "Failed to read data from file",
                INILibException::ERR_FILE_READ_WRITE_FAILED
            );
        }

        // Parse data into data array
        $result = parse_ini_string($fileContents, true, $parScannerMode);
        if ($result === false) {
            $this->fileObject->flock(LOCK_UN);
            throw new INILibException(
                "Failed to parse file contents",
                INILibException::ERR_INI_PARSE_FAILED
            );
        }
        $this->iniDataArray = $result;

        // Unlock the file when done
        $this->fileObject->flock(LOCK_UN);
    }


    /**
     * Sets a key=value pair within an INI section header in memory.
     *
     * Data passed into parameters is validated to ensure generated INI files will be properly formatted and will not
     *   produce any parsing errors.
     *
     * Parameters are also trimmed of leading and trailing whitespace prior to validation for INI formatting purposes
     *   (I.e., saving '[section]' instead of '[ section ]', or 'key=value' instead of 'key = value'. This allows for
     *   better consistency in writing and reading of INI files between this class, parse_ini_* functions, and any other
     *   programs written in other languages that may need to access these files.
     *
     * @param string $parSection INI section
     * @param string $parKey INI key
     * @param string $parValue Desired new value
     * @throws INILibException if any parameters do not fit proper INI formatting or would cause INI parsing errors if
     *   saved to a file
     * @uses INILib::$iniDataArray
     */
    public function setKey(string $parSection, string $parKey, string $parValue)
    {
        // Trim whitespace
        $parSection = trim($parSection);
        $parKey     = trim($parKey);
        $parValue   = trim($parValue);

        // Parameter validations
        // As [ and ] are 'control' characters for sections, they shouldn't exist in section names
        if ((strpos($parSection, '[') !== false) or (strpos($parSection, ']') !== false)) {
            throw new INILibException(
                "Parameter 1 (section name) cannot contain the characters '[' or ']'",
                INILibException::ERR_INVALID_PARAMETER
            );
        }
        // For similar reasons as above, a key name should not start with [
        if (substr($parKey, 0, 1) == '[') {
            throw new INILibException(
                "First character of parameter 2 (key name) cannot be '['",
                INILibException::ERR_INVALID_PARAMETER
            );
        }
        // A key name should also not contain =, as this is a control character that separates key from value
        if (strpos($parKey, '=') !== false) {
            throw new INILibException(
                "Parameter 2 (key name) cannot contain the character '='",
                INILibException::ERR_INVALID_PARAMETER
            );
        }
        // Section and key should not start with ; or #, as these are used to denote comments. Handling of comments is
        //  outside the scope of this class.
        if ((substr($parSection, 0, 1) == '#') or (substr($parSection, 0, 1) == ';') or (substr($parKey, 0, 1) == '#') or (substr($parKey, 0, 1) == ';')) {
            throw new INILibException(
                "First character of parameters 1 (section name) and 2 (key name) cannot be '#' or ';'",
                INILibException::ERR_INVALID_PARAMETER
            );
        }

        // Modify the data array
        $this->iniDataArray[$parSection][$parKey] = $parValue;
    }


    /**
     * Deletes a key=value pair from a specified section header in memory
     *
     * Parameters are trimmed of leading and trailing whitespace using trim() for consistency with the functionality of
     *   $this->setKey()
     *
     * @param string $parSection INI section
     * @param string $parKey INI key
     * @uses INILib::$iniDataArray
     */
    public function deleteKey(string $parSection, string $parKey)
    {
        // Trim whitespace
        $parSection = trim($parSection);
        $parKey     = trim($parKey);

        // Omitting parameter validations - As this method only deletes existing entries, any invalid section or key
        //  names will just have no effect.

        // Modify the data array
        if (!empty($this->iniDataArray[$parSection][$parKey])) {
            unset($this->iniDataArray[$parSection][$parKey]);
        }
    }


    /**
     * Saves configuration data from memory into the INI file
     *
     * @throws INILibException If the file could not be locked, or if there was some other failure with write operations
     * @uses INILib::$fileObject
     * @uses INILib::generateFileContent()
     */
    public function saveData()
    {
        // Check if file is writable
        if ($this->fileObject->isWritable() === false) {
            throw new INILibException(
                "File is not writable. Did you set the SplFileObject's open mode?",
                INILibException::ERR_FILE_NOT_WRITABLE
            );
        }

        // Lock the file for writing
        if ($this->fileObject->flock(LOCK_EX) === false) {
            throw new INILibException(
                "Failed to acquire an exclusive file lock",
                INILibException::ERR_FILE_LOCK_FAILED
            );
        }

        // Set pointer to start of file
        $this->fileObject->rewind();

        // Clear current file contents
        if ($this->fileObject->ftruncate(0) === false) {
            $this->fileObject->flock(LOCK_UN);
            throw new INILibException(
                "Failed to clear current data",
                INILibException::ERR_FILE_READ_WRITE_FAILED
            );
        }

        // Generate formatted INI file content and write to file
        if ($this->fileObject->fwrite($this->generateFileContent()) === null) {
            $this->fileObject->flock(LOCK_UN);
            throw new INILibException(
                "Failed to write data to file",
                INILibException::ERR_FILE_READ_WRITE_FAILED
            );
        }

        // Unlock the file when done
        $this->fileObject->flock(LOCK_UN);
    }


    /**
     * Generates a formatted string of INI data, primarily used for writing to INI files
     *
     * @return string The formatted string of INI data
     * @uses    INILib::$iniDataArray
     * @used-by INILib::saveData()
     */
    public function generateFileContent()
    {
        // Convert data array to formatted INI string
        $iniString = '';
        foreach ($this->iniDataArray as $section => $keyPair) {
            $iniString .= "[{$section}]" . PHP_EOL;

            foreach ($keyPair as $key => $value) {
                $iniString .= "{$key}={$value}" . PHP_EOL;
            }

            // Extra line break after sections for readability purposes
            $iniString .= PHP_EOL;
        }

        return $iniString;
    }
}

The INILibException class:

This extends the Exception class, and primarily exists to provide static error codes, and ensure that an error code is used, even if it's just 'ERR_UNDEFINED'. The idea behind this is it's easier to allow a program to check for specific, predefined error codes and use the message parameter to provide a human readable, more descriptive error message for logging (Or displaying in a dev environment)

<?php
/**
 * INI file parsing and manipulation library.
 *
 * @author SierraKomodo
 * @license GPL3
 */

namespace SierraKomodo\INILib;

/**
 * Extension of \Exception to provide predefined 'exception codes' for debugging/error handling purposes
 *
 * @package SierraKomodo\INIController
 */
class INILibException extends \Exception
{
    // Error code constants
    const NO_ERR                     = 0;
    const ERR_FILE_LOCK_FAILED       = 1;
    const ERR_FILE_READ_WRITE_FAILED = 2;
    const ERR_INI_PARSE_FAILED       = 3;
    const ERR_INVALID_PARAMETER      = 4;
    const ERR_FILE_NOT_WRITABLE      = 5;
    const ERR_FILE_NOT_READABLE      = 6;
    const ERR_UNDEFINED              = -1;


    /**
     * INILibException constructor.
     *
     * The only difference from Exception::__construct() is the default for parameter $code being set to
     *   self::ERR_UNDEFINED
     *
     * @param string $message
     * @param int $code
     * @param \Throwable|null $previous
     */
    public function __construct($message = "", $code = self::ERR_UNDEFINED, \Throwable $previous = null)
    {
        parent::__construct();
    }
}

Some example usage:

<?php
use SierraKomodo\INILib\INILib;

// Fetching database information from a config.ini file
$file     = new \SplFileObject(__DIR__ . DIRECTORY_SEPARATOR . "config.ini");
$iniFile  = new INILib($file);
$db       = new PDO("mysql:host={$iniFile->fetchEntry('Database', 'sDbHost')}; dbname={$iniFile->fetchEntry('Database', 'sDbName')}; charset={$iniFile->fetchEntry('Database', 'sCharset')}", $iniFile->fetchEntry('Database', 'sUser'), $iniFile->fetchEntry('Database', 'sPassword');

// Loading user-specific configuration options
$file   = new \SplFileObject(PROJECT_ROOT . DIRECTORY_SEPARATOR . $userID . DIRECTORY_SEPARATOR . "config.ini");
$config = new INILib($file);

// Fetching a specific section from a .ini file where there are multiple sections containing the same data (I.e., if the above example used a single .ini file and put each user in a separate section)
$file   = new \SplFileObject(__DIR__ . DIRECTORY_SEPARATOR . "config.ini");
$config = new INILib($file);

if ($config->fetchEntry($userID, 'key') === NULL) {
    // User doesn't have an entry, error handling or default section construction here
} else {
    // Fetch and use the user specific config data from $config->fetchEntry($userID, 'KeyNameHere')
}

I'm looking to get some feedback specifically on formatting, documentation, my idea for the extended Exception class, and ways I could improve on writing these classes. I do already have a couple ideas for additional functionality I could add (Such as $parKey being optional fetchEntry, or adding a fetchSection method; I'm also considering allowing you to pass a filepath into the constructor and instantiating the SplFileObject from within INILib instead of passing an instantiated object through the constructor). On thinking things through, static methods don't seem entirely useful outside of single write operations, since any reading can be done using the built in INI functions if you don't want to use an INILib object.

Answer 1

This looks greatly improved over previous iteration.

---

I am not a fan of naming your class the same as your library. This class is not a library unto itself, so perhaps there is more specific/meaningful name IniFile, IniIo, IniConfig, or similar might be more appropriate and descriptive than INILib. Also, a very small stylistic thing - it is most common in camel-casing class/function names to drop case all but leading characters in acronyms (for example IniLib rather than INILib). This is so that it is clear where acronyms end and "new words" begin. Spl* and Pdo* classes are common examples of this in PHP.

---

You should provide doc blocks for your class constants.

@var int {optional description}
const NO_ERR = 1;

---

const FETCH_STRING = 1;
const FETCH_MIXED  = 2;

These do not seem to be used. Remove them.

---

Should you support caller-provided setting for parsing behavior (i.e. INI_SCANNER_NORMAL)? This means the caller got get different datsa structure renderings which are potentially unreliable to other areas of the system. Should you consider INI_SCANNER_TYPED as default parsing behavior, as this would tend to yield data values in your array that are more meaningful to be used in other areas of the application (i.e. booleans vs. 'yes', 'no', 'on', 'off', etc.). This could of course be a problem for persisting to file as you would need to potentially have a conversion back to 'yes', 'no', etc.

Perhaps getArray() functionality needs to make boolean conversions? I guess my summary thought here is that you are potentially introducing inconsistent conversion behavior with different parsing modes, and I don't really see this addressed in your code.

---

Some suggestions on method naming for consistency/clarity:

• Is getData() or getDataArray() and better method name than dataArray()?
• getEntry() instead of fetchEntry()?
• setEntry() instead of setKey()? (Why call it "entry" in one place and "key" in another?)
• deleteEntry() instead of deleteKey()?
• saveDataToFile, updateFile(), or similar instead of saveData() (something that makes is clear you are writing to the INI file)?
• formatDataForOutput() or similar rather than generateFileContent(). I guess my sticking pooint here is in the word "generate" which might convey more, behavior-wise, than what this method really does.

---

Should parseIniData() be public? What is use case for caller needing this method if this is automatically done from constructor (which I think is the proper approach)?

Should generateFileContent() be public? I don't see how it is meaningful for caller to access this method.

---

Sections in this class only seem to be considered as compound keys towards individual config line entries. Should this class consider section-level get/delete methods (perhaps set as well, if you are OK with potentially creating empty sections).

Similarly should setKey() be allowed to have the capability to create arbitrary sections as it currently does? (I don't think that there is a right or wrong here. This really depends on how you intend to use the class and INI files.)

---

In setKey() method you have a lot of validation that might best be moved into validateConfigKey(), validateSectionKey() or similar methods. You may consider regex as part of your validation methodology, to more concisely cover your validation cases. You are also missing some other cases around allowable config key names (i.e. reserved words as noted in parse_ini_string() documentation) that you may want to include as part of validation so as to avoid unexpected behavior.

You could reuse these validations elsewhere (like on get/set key/entry methods).

---

In setkey() you might consider not trimming input, but rather making sure that looking for whitespace at edges of string are part of validation. Do you really want to enable the caller to send you a malformed string, with the class silently correcting them vs. having the caller send correct strings?

---

Do you really need to lock the file for reads? If you want to maintain this, you should unlock the file immediately after you make the read, vs. in various places further in the parse method (you have the same line repeated multiple times, which can be avoided).

i.e.

$fileContents = ...;
$this->fileObject->flock(LOCK_UN);
// rest of code

---

Does it make sense to check if a file writable in the constructor (i.e. long before a "save" method is ever called), or is there perhaps a reason to support read-only "mode" of operation. If so, should read-only be a setting at time of instantiation, such that Exception can be thrown to caller that they are trying to write for a read-only INI file? I guess my thinking here is that the class is potentially being put into a bad state that can either be detected up front, or more specifically addressed in the interface.

I also note that you have no place where you are validating readability of the file. Should this happen in constructor or at beginning of parse method before you try to operate against the file?

---

In save method, it seems that ftruncate() and rewind() are perhaps reversed in order. I don't know that it really matters, but there is no reason to rewind before truncation, and it seems more logical to place rewind call directly before you are attempting to do write. There is no reason to even rewind the file pointer if you cannot successfully truncate the file, right?

---

Include @return void in doc blocks for methods that have no return (with exception of constructor).

---

You need to pass parameters from your exception constructor to parent::__construct().

---

You should be using \Exception, not \Throwable as type hint for third parameter (you are breaking the contract of parent class).

---

I don't understand NO_ERR as one of your exception codes. What is this for? When would you throw an exception with this code? Exception codes are not like exit codes where 0 means success, in fact, the vast majority of exceptions you will probably encounter only use code 0.