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

Responsibilities of index.php and what to do about it #1001

Closed
31 tasks done
logmanoriginal opened this issue Jan 8, 2019 · 1 comment
Closed
31 tasks done

Responsibilities of index.php and what to do about it #1001

logmanoriginal opened this issue Jan 8, 2019 · 1 comment
Assignees
Labels
Code-Investigation-Needed Code needs to be investigated Code-Refactoring Code refactoring Need-Info Additional information required question Indicates that an issue or pull request needs more information

Comments

@logmanoriginal
Copy link
Contributor

logmanoriginal commented Jan 8, 2019

index.php has grown quite complex with the addition of new features and continuous effort of the community for improvement. From my point of view it has reached a point where it makes sense to re-factor it into smaller pieces. A task I'm currently working on.

This issue is about making the process transparent for everyone and to get some feedback before investing too much of my precious spare time. It also serves as a TODO list for all the things that should be done.

But going into that, find below my summary for the current responsibilities of index.php (basically a human friendly interpretation of the file):

Show responsibilities of `index.php`
  • Set default time zone to UTC
  • Append CLI arguments to the $_GET array
  • Define the user agent
  • Specify the default whitelist
  • Handle actions
    • Handle the list action
      • Calculate output data
      • Collect output data
      • Set the Content-Type header
      • Send output data to the user
    • Handle the detect action
      • Find target bridge
      • Run the operation
      • Build output data
      • Set the Location header
      • Send output data to the user
    • Handle the display action
      • Cleanup format input
      • Check if the bridge is whitelisted
      • Set the proxy flags
      • Handle custom cache timeout
      • Redirect if custom cache timeout is disabled
      • Filter cache timeout value
      • Filter bridge parameters
      • Filter cache parameters
      • Initialize cache
      • Handle cache timeout
        • If it's not a timeout load cached data
          • Check Http-If-Modified-Since header
          • Interpret header data
          • Send Last-Modified header
          • Load cached data
        • If it's a timeout load new data
          • Handle errors and exceptions
            • If its' an error
              • Generate a feed item to return as error item
              • Collect additional data
              • Add additional data to the error item
              • Add the item to the items list
            • If its' an exception
              • Generate a feed item to return as error item
              • Collect additional data
              • Add additional data to the error item
              • Add the item to the items list
          • Initialize the bridge
          • Handle legacy items
          • Collect bridge information
          • Build cache storage data
        • Transform data
          • Handle errors and exceptions
            • If it's an error
              • Write error message to error_log
              • Send Content-Type header
              • Kill the process, sending error data to the user
            • If it's an exception
              • Write error message to error_log
              • Send Content-Type header
              • Kill the process, sending error data to the user
    • Echo the bridge list to the user
  • Handle exceptions
    • Write error message to error_log
    • Send Content-Type header
    • Kill the process, sending error data to the user

Based on the list of responsibilities I came up with a list of tasks that take the SOLID approach a little more serious.

The SOLID approach

S = Single responsibility principle
O = Open/closed principle
L = Liskov substitution principle
I = Interface segregation principle
D = Dependency inversion principle

=> Google it
=> Don't take it too seriously 😉

Single responsibility principle

  • Setting the time zone is responsibility of the Configuration class. References Time stamp #956
  • Loading CLI and GET arguments should be handled by a UserInput class.
  • Setting the user agent is responsibility of the Configuration class.
  • The default whitelist is responsibility of the Configuration class.
  • index.php should work as a controller, as in MVC.
  • Actions should be handled by specialized *Action classes.
  • *Action classes should be initialized by a ActionFactory class.
  • Exceptions should be handled inside the implementing codes.
  • User input data should be cleaned up by the UserInput class.
  • Factory classes are responsible for taking any supported name.
  • Proxy flags are responsibility of the Configuration class.
  • Custom cache timeout is responsibility of the Cache class.
  • Cache initialization is responsibility of the CacheAbstract class.
  • The cache type is responsibility of the Configuration class.

Open/closed principle

// Mostly covered by previous points

  • Link actions in a factory class instead of using a case structure
  • Implement a base class for factory classes
  • Add separate classes for each action
    • Display
    • List
    • Detect

Liskov substitution principle

  • Each element that can be changed by the user should implement an interface.
    • Bridges
    • Caches
    • Formats
    • Actions
  • Refactor existing factories

Interface segregation principle

// Mostly covered by previous points

Dependency inversion principle

// Mostly covered by previous points

Testing

  • It must be possible to test the (each) API in isolation.
    • Bridges
    • Caches
    • Formats
    • Actions

Let me know if you have further suggestions or want to work on any of the points listed above.

@logmanoriginal logmanoriginal added Code-Refactoring Code refactoring question Indicates that an issue or pull request needs more information Need-Info Additional information required Code-Investigation-Needed Code needs to be investigated labels Jan 8, 2019
@logmanoriginal logmanoriginal self-assigned this Jan 8, 2019
logmanoriginal added a commit that referenced this issue Jun 6, 2019
Default bridges are currently statically defined in index.php, which
is not the right place if we want to keep responsibilities separated.

This commit introduces a new file whitelist.default.txt that holds
the default bridges and which is loaded automatically, if whitelist.txt
doesn't exist.

Due to this it is also no longer necessary to have write permission
for the root directory.

References #1001
logmanoriginal added a commit that referenced this issue Jun 7, 2019
RSS-Bridge currently statically sets the timezone to UTC which can
result in incorrect timestamps if the server is hosted in another
region.

This commit adds a new configuration parameter to allow admins to
specify their own timezone for their servers. Invalid values will
result in an error message.

Example:

  [system]
  timezone = "UTC"

For compatibility reasons the default value is set to UTC.

This parameter accepts any of the supported timezones listed at
https://www.php.net/manual/en/timezones.php

Closes #956
References #1001
logmanoriginal added a commit that referenced this issue Jun 18, 2019
RSS-Bridge currently sanitizes the format name only for the display
action, which can cause problems if other actions depend on formats
as well.

It is therefore better to do sanitization in the factory class for
formats. Additionally, formats should not require a perfect match,
so 'Atom' and 'aToM' make no difference. This will also allow users
to define formats in their own style (i.e. only lowercase via CLI).

References #1001
logmanoriginal added a commit that referenced this issue Jun 18, 2019
The bridge factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the bridge factory.

References #1001
logmanoriginal added a commit that referenced this issue Jun 18, 2019
The cache factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the factory.

References #1001
logmanoriginal added a commit that referenced this issue Jun 18, 2019
The format factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the factory.

References #1001
@logmanoriginal
Copy link
Contributor Author

All done.

infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this issue Apr 17, 2020
Default bridges are currently statically defined in index.php, which
is not the right place if we want to keep responsibilities separated.

This commit introduces a new file whitelist.default.txt that holds
the default bridges and which is loaded automatically, if whitelist.txt
doesn't exist.

Due to this it is also no longer necessary to have write permission
for the root directory.

References RSS-Bridge#1001
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this issue Apr 17, 2020
RSS-Bridge currently statically sets the timezone to UTC which can
result in incorrect timestamps if the server is hosted in another
region.

This commit adds a new configuration parameter to allow admins to
specify their own timezone for their servers. Invalid values will
result in an error message.

Example:

  [system]
  timezone = "UTC"

For compatibility reasons the default value is set to UTC.

This parameter accepts any of the supported timezones listed at
https://www.php.net/manual/en/timezones.php

Closes RSS-Bridge#956
References RSS-Bridge#1001
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this issue Apr 17, 2020
RSS-Bridge currently sanitizes the format name only for the display
action, which can cause problems if other actions depend on formats
as well.

It is therefore better to do sanitization in the factory class for
formats. Additionally, formats should not require a perfect match,
so 'Atom' and 'aToM' make no difference. This will also allow users
to define formats in their own style (i.e. only lowercase via CLI).

References RSS-Bridge#1001
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this issue Apr 17, 2020
The bridge factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the bridge factory.

References RSS-Bridge#1001
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this issue Apr 17, 2020
The cache factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the factory.

References RSS-Bridge#1001
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this issue Apr 17, 2020
The format factory can be based on the abstract factory class if it
wasn't static. This allows for higher abstraction and makes future
extensions possible. Also, not all parts of RSS-Bridge need to work
on the same instance of the factory.

References RSS-Bridge#1001
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code-Investigation-Needed Code needs to be investigated Code-Refactoring Code refactoring Need-Info Additional information required question Indicates that an issue or pull request needs more information
Projects
None yet
Development

No branches or pull requests

1 participant